Support line continuation with backslashes

[bpeel]

Adds support for continuing long lines with backslashes in the scripts. For example:

colorWriteMask VK_COLOR_COMPONENT_R_BIT | \
               VK_COLOR_COMPONENT_G_BIT | \
               VK_COLOR_COMPONENT_B_BIT

[jaebaek]

This mostly looks good. I want to discuss three issues with you:

1. Could you please add a description for this in README.md?
2. I am suddenly worried its performance. Here, it uses fgetc() that would be much slower than fgets() that we previously used. Maybe many file systems have an internal cache to make fgetc() almost the same as fgets() in the performance and it would be fine. If you have an idea to make it as batch processing, it would be nice.
3. Many vr_buffer_append_c have the same issue. Increasing the buffer size one by one would be inefficient.

If the first issue is solved, then I think this pull request is good to be merged.

However, we need to consider performance issues in long term (not only this PR). I wonder that VkRunner is fast enough for CTS. We need some ways to measure the performance and improvements e.g., doubling the capacity of buffer for every increment whatever the given size is. Let's discuss this issue when we have the real performance measurement.

[bpeel]

Could you please add a description for this in README.md?

Ok, I have added this.

Here, it uses fgetc() that would be much slower than fgets() that we previously used.

I think the whole point of FILE* is that it buffers the I/O so as far as I understand fgetc should be relatively fast. At any rate I’d be surprised if loading the script is a significant percentage of the execution time of the test so it might not be the most important thing to optimise at the cost of maintainability.

Many vr_buffer_append_c have the same issue.

vr_buffer_append_c already does double the size of the buffer whenever it is full so it should only very rarely need to realloc. In the case where it doesn’t realloc it is an inline function that just directly puts the character in the buffer so it should be very cheap.

Thanks for looking at the branch.

[bpeel]

I made this hacky patch to time reading all of the lines in the script a million times. On master on my laptop this takes ~3.4 seconds and on this branch it takes approximately ~13.8 seconds. So I guess you are right that this approach is significantly slower. On the other hand effectively this means that if you did a test run of a million tests the branch will only add 10.4 seconds to the total running time, so I don’t know if it really matters.

It might be difficult to optimise without significantly diverging the codepaths for handling files and strings so I think it might be better to optimise later when we have a proper unit test framework to ensure we don’t break things.

[bpeel]

I’ve changed the branch to go back to using fgets instead. In the end I think the code doesn’t diverge as much as I thought between FILE and string streams. This version no longer handles Mac '\r' line endings, but apparently that is only OS 9 so it’s probably not relevant.

[jaebaek]

Thank you for checking the performance! Mostly looks good to me.

One minor issue is error message for parsing the VkRunner script. When a line of the script has an invalid test command, the reported line number will be different from its real line number because we merged lines.

The attached patch solves this problem. Please take a look and adopt the idea. I changed the files and ran git diff > multines.txt.

multilines.txt

[bpeel]

Thanks for the suggestion. I’ve incorporated it and merged the branch.