-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Log properties and items on ProjectEvaluationFinished (#6287)
Add an option to log global properties, properties and items on ProjectEvaluationFinishedEventArgs instead of ProjectStartedEventArgs. This option is currently only turned on by the BinaryLogger. This has several advantages. Currently only the projects that are built by the central node log their properties and items (properties are translated across nodes only if a special flag is set, and items are never translated). This resulted in properties and items not being available for projects built on other nodes. Now we log them after every evaluation and translate across nodes if needed. Together with the fact that we now log EvaluationId for each ProjectStarted, we can now recover properties and items for all project started events. This is the main purpose of this PR - to not lose properties and items like we currently do. We will still not log for project results that are satisfied by cache, because we don't keep track of evaluation for these. Presumably it will have already been logged previously. In addition, if more than one project are built from the same evaluation, we do not duplicate properties and items, only logging them once. This results in logging more information, but storing it more efficiently. Together with string and dictionary deduplication we see very significant savings in binlog size and some reduction in build time. This change has several large parts: 1. add a way to enumerate evaluation properties and items directly at the end of Evaluate() for PropertyDictionary<ProjectPropertyInstance> and ItemDictionary<ProjectItemInstance> 2. manual translation logic for ProjectEvaluationStarted and ProjectEvaluationFinished (instead of relying on TranslateDotNet/BinaryFormatter) 3. reading and writing ProjectEvaluationFinished GlobalProperties, Properties and Items in BuildEventArgsReader/Writer (used by BinaryLogger) 4. adding IEventSource4 with IncludeEvaluationPropertiesAndItems, to propagate this setting across nodes and threading it through the LoggingService 5. update the ParallelConsoleLogger and SerialConsoleLogger to print the new data, if present 6. tests One controversial design decision here is storing a reference to live evaluation data in ProjectEvaluationFinishedEventArgs. It does not make a snapshot of the data to avoid very significant allocations. It does take the lock on the PropertyDictionary<T>/ItemDictionary<T> when enumerating, because logging is asynchronous and the logging consumer (BinaryLogger) will enumerate the data potentially after the build has already started and the data is being mutated. I did see exceptions when enumerating without the lock. We had the same problem when the data was logged on ProjectStartedEventArgs though. In addition, there's a slight risk of logging not the exact data as it was at the end of evaluation, but the mutated data after some target has modified it. However given that the previous behavior was to not log anything for out-of-proc projects, and given the very significant allocation reduction, I think it's worth it. To mitigate, we could capture a snapshot at the end of evaluation, so we don't hold a reference to live data. This won't need a lock to enumerate. Ideally we also rely on the immutable collections to avoid allocations, but I didn't see an easy way to do that currently. We can investigate this in a future change. For items, it doesn't concatenate items of different types into a single large item stream, but keeps multiple lists, one per item type, to reflect the internal representation. Not flattening item types results in savings because we don't have to mention the item type for each item. This change increments the BinaryLogger file format to 12, to serialize GlobalProperties, Properties and Items on ProjectEvaluationFinishedEventArgs. It also stores items more efficiently, without having to know the number of item types in advance and enumerate in a single pass. It writes the item type and all items of that type, and it writes 0 to signal there are no more item types. It also no longer writes the Message for args as it can be recovered upon reading. New EnumerateProperties() and EnumerateItems() methods are added to Utilities, to consolidate the logic to enumerate the new data structures in a single location, used by packet translation logic, binary logger and the console loggers. Perf wise, I'm seeing no significant change on binlog size for small builds (it's a wash, because we log properties/items for all projects now, but they are no longer duplicated). For large projects I expect very significant savings though, as ProjectStarted is the most heavy-weight event in large binlogs. Build performance with /bl on small-ish builds is improved 27 s -> 24 s for single-core and 18 s -> 17 s for parallel. No observable change without /bl. Fixes #5316 Fixes #3616
- Loading branch information
1 parent
073022e
commit 84ad007
Showing
40 changed files
with
1,420 additions
and
222 deletions.
There are no files selected for viewing
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
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
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
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
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
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
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
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
Oops, something went wrong.