incr-check enhancements, and CI for incremental test cases #21518
+337
−79
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Oh, I forgot to add: here's how long it takes to run I'm using a Debug compiler here, so the numbers will be a little inflated -- probably release builds shave 10 seconds off. However, it won't be too drastic, because it's obvious just from watching the progress tree that the vast majority of time here is just spent waiting for |
mlugg
force-pushed
the
incremental-ci
branch
4 times, most recently
from
5df7b7c
to
d37c56e
Compare
This is contained in the `test` step, so is tested by CI. This commit also includes some enhancements to the `incr-check` tool to make this work correctly.
If no external executor is available for a successful binary, its execution is silently skipped. This allows the CI to test, to the fullest extent possible, incremental cross-compilation to targets whose binaries can't be executed on the host.
The new `--preserve-tmp` flag can be used to preserve the temporary directory for debugging purposes.
Throw another target in there just to spice things up a little! Running the incremental cases with the C backend is pretty slow due to the need to recompile the whole output from scratch on every update; for this reason, we probably don't want to keep many of these targeting CBE long-term. However, for now, while we have relatively few tests and things are still changing quite a lot, it's better to have this little bit of extra test coverage.
* fix inconsistency in global cache directory name * don't error if spawning external executor fails * handle CRLF correctly
mlugg
force-pushed
the
incremental-ci
branch
from
d37c56e
to
e4ec8ef
Compare
…Windows This commit changes how `std.io.poll` is implemented on Windows. The new implementation unfortunately incurs a little extra system call overhead, but fixes a major bug in the old implementation. I'm not yet set on this implementation, but I'm pushing it to check if it resolves my CI failures. The old implementation was buggy in that by leaving `ReadFile` calls into the `LinearFifo` write end pending between `poll` calls, there was potential for a race condition where the kernel satisfies the asynchronous read after the caller has modified the corresponding `LinearFifo` in some way; most likely by rotating its buffer for a `readableSliceOfLen` call. The only way to allow the full `LinearFifo` API to be used is to not leave these `ReadFile` calls pending after `poll` returns. So, now, `pollWindows` will leave running a different set of `ReadFile` calls, which are trying to read a single byte into a temporary buffer. These are the operations we wait on with `WaitForMultipleObjects`. When these reads are completed, we push that single byte to the FIFO, and perform some reads directly into the FIFO memory until we stop getting immediate results; then, we again queue a single-byte read into a stable buffer for the next call to `poll`. My main concern with this implementation is that if the kernel frequently returns `IO_PENDING` for `ReadFile` calls even when the data is available, then we could effectively skip the bulk-reading path, hence reading data one byte at a time. Performance measurements are necessary here.
This reverts commit e4ec8ef.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This does exactly what the title says.
zig build testnow includeszig build test-incremental, which runsincr-checkover every case intest/incremental/(we expect exit code 0 fromincr-check). In addition,incr-checkundergoes several enhancements:incr-checkis removed by default (both on success and on error); it can be retained for debugging purposes with the--preserve-tmpflagAll of the incremental test cases now include the
x86_64-windows-cbetarget too, just for a little more coverage -- see the corresponding commit message.