cli/printlog: improve compact exec log #6350
Conversation
cli/printlog/compact/compact.go
Outdated
| } | ||
| spawnInputs = append(spawnInputs, file) | ||
| } | ||
| if slr.sort { |
There was a problem hiding this comment.
The old exec format didn't sort the individual fields of the spawn proto as those are already deterministically ordered (based on depset order). Instead, to make the log more comparable overall, we would need to sort the protos with respect to each other.
Bazel topologically sorts by consumer-producer relationship and then by output path, see https://github.com/bazelbuild/bazel/blob/5d4feefed7e39b20a6c5deb5f74394abbf622a52/src/main/java/com/google/devtools/build/lib/exec/StableSort.java#L55. I think that this is what we should do if -sort is specified.
There was a problem hiding this comment.
Im a bit afraid of implementing this.
Currently, we are converting from the new compact format to the old format, which is big and could take a long time to sort.
But perhaps it's fine because we are not processing the log inside the invocation. 🤔
Let me give it a try.
There was a problem hiding this comment.
I don't think it's possible to sort the new compact format directly as consumer/producer relationships can only be discovered after you have "flattened" the input/output file sets. If you want, we could take a look at this together at some point and see whether we can do better.
There was a problem hiding this comment.
I have removed the --sort for now.
This PR is now a lot smaller to fix obvious bugs and add some minor doc improvements.
PTAL.
sluongng
force-pushed
the
sluongng/compact-exec-improvements
branch
from
73ac678
to
360bf29
Compare
Fixed an issue in `reconstructDir()` where the filesInDir slice was initiated with nil entries. Rename the compact log file to have the right extension.
sluongng
force-pushed
the
sluongng/compact-exec-improvements
branch
from
360bf29
to
bbc85ca
Compare
Fixed an issue in
reconstructDir()where the filesInDir slice wasinitiated with nil entries.
Rename the compact log file to have the right extension.