Client, refactor: simplify nullary flag handling #9121
Closed
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
Nullary startup flags (e.g. --[no]master_bazelrc) are registered with StartupOptions::RegisterNullaryStartupOption. Until now, nullary flags also had their special handling logic, even though the logic was always the same. Now, nullary flags are registered together with their member variable's pointer, and there's only one code path to handle all. Startup flags forbidden in .bazelrc files are handled specially. Until now, we searched the handler code linearly via a long if-elseif-elseif-... chain, so parsing N flags out of M possible flags performed O(N*M) string comparisons. Now, we look up the nullary flag value from a std::unordered_map, so the code is faster which has O(1) lookup complexity, reducing the total complexity O(M). Change-Id: Ie3dcba4ae6afa959e84657b7ff9bb8f4c87777fd
laszlocsomor
changed the title
Change-Id: I74feb2936bb495259a072f1212b240c2ee62b6e4
Change-Id: I794cdc553ea833620b174375fbfdaa7492113d13
laszlocsomor
changed the title
meteorcloudy
approved these changes
Aug 8, 2019
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.
Nullary startup flags (e.g. --[no]master_bazelrc)
are registered with
StartupOptions::RegisterNullaryStartupOption.
Important change:
--host_jvm_debugnow supportsthe negative flag (
--nohost_jvm_debug).I suspect the missing support was a bug resulting
from the code duplication this PR eliminates.
Until now, nullary flags also had their special
handling logic, even though the logic was always
the same.
Now, nullary flags are registered together with
their member variable's pointer, and there's only
one code path to handle all. Startup flags
forbidden in .bazelrc files are handled specially.
Until now, we searched the handler code linearly
via a long if-elseif-elseif-... chain, so parsing
N flags out of M possible flags performed O(N*M)
string comparisons.
Now, we look up the nullary flag names from a
std::unordered_map, which has O(1) lookup
complexity, reducing the total complexity to O(M).
Also a drive-by improvement in blaze.cc: use
std::unique_ptr instead of manual new/delete.
Change-Id: Ie3dcba4ae6afa959e84657b7ff9bb8f4c87777fd