Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log properties and items on ProjectEvaluationFinished #6287

Merged
merged 7 commits into from Apr 2, 2021

Commits on Mar 20, 2021

  1. Log properties and items on ProjectEvaluationFinished

    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.
    KirillOsenkov committed Mar 20, 2021
    Copy the full SHA
    79b4f41 View commit details
    Browse the repository at this point in the history

Commits on Mar 27, 2021

  1. Copy the full SHA
    0a517ea View commit details
    Browse the repository at this point in the history
  2. Fix name.

    KirillOsenkov committed Mar 27, 2021
    Copy the full SHA
    ffb63ca View commit details
    Browse the repository at this point in the history

Commits on Mar 30, 2021

  1. Update src/Build/Collections/ItemDictionary.cs

    Co-authored-by: Forgind <Forgind@users.noreply.github.com>
    KirillOsenkov and Forgind committed Mar 30, 2021
    Copy the full SHA
    ad644b2 View commit details
    Browse the repository at this point in the history
  2. Update src/Build/Utilities/Utilities.cs

    Co-authored-by: Forgind <Forgind@users.noreply.github.com>
    KirillOsenkov and Forgind committed Mar 30, 2021
    Copy the full SHA
    f953c0d View commit details
    Browse the repository at this point in the history
  3. Revert "Update src/Build/Collections/ItemDictionary.cs"

    This reverts commit ad644b2.
    KirillOsenkov committed Mar 30, 2021
    Copy the full SHA
    87d1d51 View commit details
    Browse the repository at this point in the history
  4. Use switch expression.

    KirillOsenkov committed Mar 30, 2021
    Copy the full SHA
    b7f5607 View commit details
    Browse the repository at this point in the history