Optimize builtins restriction check #28090
Conversation
`BuiltinRestriction` currently accounts for 9% of the total CPU time for `bazel build --nobuild //src:bazel-dev`. This is avoided by replacing the linear search with a sorted map lookup and avoiding the repeated creation of "non-visible" `RepositoryName` objects, which trigger a `SpellChecker` computation. The map lookup is made possible by the transition to Bzlmod, which makes it possible to drop legacy repo names and treat the remaining apparent repo names as module names, for which it is easy to derive a prefix of the canonical repo name.
|
@bazel-io fork 9.0.0 |
fmeum
requested review from
Wyverald and
gregestren
and removed request for
a team,
gregestren and
lberki
github-actions
bot
added
team-Configurability
| } | ||
|
|
||
| final boolean allows(Label label, RepositoryMapping repoMapping) { | ||
| return label.getRepository().equals(repoMapping.get(apparentRepoName())) |
There was a problem hiding this comment.
could you check if we can achieve similar performance gains by just replacing this line with a reposMatch call (and dropping all the prefix checking etc)?
It seems to me that the majority of the performance gain should be coming from treating apparentRepoName as effectively moduleName (plus weird cases like _builtins and the main repo). The sorted map, flooring, etc etc introduce a lot of extra complexity for (what I'm guessing is) comparatively little gain.
There was a problem hiding this comment.
Instead of a 10-20% slowdown, that approach results in only a 3% slowdown: #28173
That is a drastic improvement already and we can review and merge that PR separately, but I think that we should still improve this further. A 3% slowdown is still pretty bad for something with no benefit to the end user, so I would want to improve this further.
2 participants
BuiltinRestrictioncurrently accounts for 9% of the total CPU time forbazel build --nobuild //src:bazel-dev. This is avoided by replacing the linear search with a sorted map lookup and avoiding the repeated creation of "non-visible"RepositoryNameobjects, which trigger aSpellCheckercomputation.The map lookup is made possible by the transition to Bzlmod, which makes it possible to drop legacy repo names and treat the remaining apparent repo names as module names, for which it is easy to derive an unambiguous prefix of the canonical repo name.