Performance improvements RecomputeOutputsDirtyCache - #2821
Open
moritzx22 wants to merge 5 commits into
Open
Conversation
std::equal(a.begin(), a.end(), b.begin()) doesn't verify that a and b are the same length. Use the 4-iterator overload instead.
Skip the allocation entirely for phony edges, which never use it. Moved from the constructor into all(), right before first use, so instances that are constructed but never have all() called on them also avoid the allocation.
Many edges have exactly one output. Cache the lookup result in an member (logEntryOneOutput_) for that case instead of allocating a std::vector, for better performance.
| private: | ||
| bool evaluated_ = false; | ||
| BuildLog::LogEntry* entry_ = nullptr; | ||
| static BuildLog::LogEntry* const kUnevaluated; |
Contributor
There was a problem hiding this comment.
nit: Just use a constant pointer value like reinterpret_cast<const BuildLog::Entry*>(1) instead of loading a pointer for .rodata on each call.
You cannot make such an expression constexpr due to the reinterpret_cast<> so, as a special case, just use a macro for simplicity instead. Call this CACHED_LOG_ENTRY_INIT_SENTINEL for clarity.
| // the log against the most recent input's mtime (see below) | ||
| bool used_restat = false; | ||
| if (isRestat_ && buildLog_ && entry.LookupByOutput(buildLog_, output)) { | ||
| if (isRestat_() && buildLog_ && entry.LookupByOutput(buildLog_, output)) { |
Contributor
There was a problem hiding this comment.
So much complexity makes the code unnecessarily obscure and hard to follow, Just add a is_restat_init_ boolean flag, and a is_restat() method that will check it before returning the result to perform.
Follow conventions by using snake_case and not pascalCase.
entry_ now starts pointing at a static object instead of nullptr, so a single pointer tracks both "not yet evaluated" and "evaluated, not found". No separate evaluated_ flag needed anymore. Shrinks CachedLogEntry from 16 to 8 bytes, halving the size of the logEntry_ vector. Most noticeable on edges with many outputs.
moritzx22
force-pushed
the
RecomputeOutputsDirtyCache_Improve
branch
from
August 11, 2026 17:52
3682921 to
7d679ac
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 PR adds several performance improvements to class
RecomputeOutputsDirtyCache, see PR #2680. There are no functional changes.logEntry_isRestat_CachedLogEntryfrom 16 to 8 bytes