Skip to content

Remove throwaway builders, boxed positions and eager maps from the parse path - #1221

Open
copybara-service[bot] wants to merge 1 commit into
mainfrom
test_979625851
Open

Remove throwaway builders, boxed positions and eager maps from the parse path#1221
copybara-service[bot] wants to merge 1 commit into
mainfrom
test_979625851

Conversation

@copybara-service

Copy link
Copy Markdown
Contributor

Remove throwaway builders, boxed positions and eager maps from the parse path

Four related allocation reductions. They land together because the last two are
coupled: the parser's new one-shot position map only pays off once the source
builder can adopt an immutable map instead of copying it.

CelExpr. Every newBuilder() allocated a fresh CelNotSet plus an ExprKind wrapper
purely to be overwritten a moment later; those become shared NOT_SET_KIND and
NOT_SET_EXPR singletons. CelSelect paid that once per node and CelComprehension
five times. addArgs, addElements and addEntries used Arrays.asList(...) followed
by forEach(list::add), allocating a list view and a capturing lambda per call,
and now use Collections.addAll and Iterables.addAll. The ofCall, ofList,
ofStruct and ofMap factories hand an already-immutable collection straight to
the value class rather than copying it through the builder's mutable ArrayList.

PrattParser. Builds calls, lists, maps and structs through those factories,
which also shortens the call sites. The expression position map becomes an int[]
indexed by expression id rather than a Map<Long, Integer>; ids are dense and
handed out sequentially by nextId, so this drops two boxed objects and a hash
insert per node. copyPositionsTo then builds the map once, presized. The source
content is cached in a field rather than re-fetched through the accessor, and
nextSignificantToken loses a loop over WHITESPACE and COMMENT tokens that could
never iterate, since Lexer.lex() consumes those internally and never emits them.

CelSource.Builder. positions and macroCalls start as empty immutable maps and
are copied into a HashMap only when something actually mutates them. Two
builders exist per parse, so this removes up to four hash maps. addPositionsMap
and addAllMacroCalls adopt an already-immutable argument outright while the
builder is still pristine, which makes build()'s copyOf a no-op and removes the
second copy of the parser's position map. extensions is created lazily, and
addAllExtensions short-circuits on an empty argument, which matters because
toBuilder() always calls it.

CelValidationResult. Hoists its issue comparator into a constant and replaces a
stream().anyMatch(...) with an indexed loop. Every successful parse paid both.

Measured with CelParserBenchmark (parseOnly, built -c opt). "Pratt before" is
this CL's parent, so the last column is this CL's own contribution. The ANTLR
column is the series baseline and is unaffected by any of these changes.

Objects allocated per parse:

Case ANTLR Pratt before Pratt after Pratt vs ANTLR Delta this CL
SMOKE_TEST 357 120 70 5.1x smaller -41.7%
CHAINED_ORS 968 339 201 4.8x smaller -40.7%
LIST_COMPREHENSION 512 160 100 5.1x smaller -37.5%
MESSAGE_CREATION 1,253 406 272 4.6x smaller -33.0%
LONG_LIST 81,794 18,263 13,005 6.3x smaller -28.8%

Bytes allocated per parse:

Case ANTLR Pratt before Pratt after Pratt vs ANTLR Delta this CL
SMOKE_TEST 12,256 3,512 2,040 6.0x smaller -41.9%
CHAINED_ORS 32,160 9,592 5,656 5.7x smaller -41.0%
LIST_COMPREHENSION 17,320 4,736 2,928 5.9x smaller -38.2%
MESSAGE_CREATION 43,128 12,416 8,696 5.0x smaller -30.0%
LONG_LIST 2,907,488 531,888 390,992 7.4x smaller -26.5%

Wall clock, mean of 3 caliper trial medians:

Case ANTLR Pratt before Pratt after Pratt vs ANTLR Delta this CL
SMOKE_TEST 4,940 ns 692 ns 388 ns 12.7x faster -43.9%
CHAINED_ORS 14,641 ns 2,055 ns 1,111 ns 13.2x faster -45.9%
LIST_COMPREHENSION 7,514 ns 1,231 ns 797 ns 9.4x faster -35.3%
MESSAGE_CREATION 20,979 ns 3,583 ns 2,524 ns 8.3x faster -29.6%
LONG_LIST 1,616,631 ns 142,764 ns 117,823 ns 13.7x faster -17.5%

…rse path

Four related allocation reductions. They land together because the last two are
coupled: the parser's new one-shot position map only pays off once the source
builder can adopt an immutable map instead of copying it.

CelExpr. Every newBuilder() allocated a fresh CelNotSet plus an ExprKind wrapper
purely to be overwritten a moment later; those become shared NOT_SET_KIND and
NOT_SET_EXPR singletons. CelSelect paid that once per node and CelComprehension
five times. addArgs, addElements and addEntries used Arrays.asList(...) followed
by forEach(list::add), allocating a list view and a capturing lambda per call,
and now use Collections.addAll and Iterables.addAll. The ofCall, ofList,
ofStruct and ofMap factories hand an already-immutable collection straight to
the value class rather than copying it through the builder's mutable ArrayList.

PrattParser. Builds calls, lists, maps and structs through those factories,
which also shortens the call sites. The expression position map becomes an int[]
indexed by expression id rather than a Map<Long, Integer>; ids are dense and
handed out sequentially by nextId, so this drops two boxed objects and a hash
insert per node. copyPositionsTo then builds the map once, presized. The source
content is cached in a field rather than re-fetched through the accessor, and
nextSignificantToken loses a loop over WHITESPACE and COMMENT tokens that could
never iterate, since Lexer.lex() consumes those internally and never emits them.

CelSource.Builder. positions and macroCalls start as empty immutable maps and
are copied into a HashMap only when something actually mutates them. Two
builders exist per parse, so this removes up to four hash maps. addPositionsMap
and addAllMacroCalls adopt an already-immutable argument outright while the
builder is still pristine, which makes build()'s copyOf a no-op and removes the
second copy of the parser's position map. extensions is created lazily, and
addAllExtensions short-circuits on an empty argument, which matters because
toBuilder() always calls it.

CelValidationResult. Hoists its issue comparator into a constant and replaces a
stream().anyMatch(...) with an indexed loop. Every successful parse paid both.

Measured with CelParserBenchmark (parseOnly, built -c opt). "Pratt before" is
this CL's parent, so the last column is this CL's own contribution. The ANTLR
column is the series baseline and is unaffected by any of these changes.

Objects allocated per parse:

| Case | ANTLR | Pratt before | Pratt after | Pratt vs ANTLR | Delta this CL |
| :--- | ---: | ---: | ---: | ---: | ---: |
| SMOKE_TEST | 357 | 120 | 70 | 5.1x smaller | -41.7% |
| CHAINED_ORS | 968 | 339 | 201 | 4.8x smaller | -40.7% |
| LIST_COMPREHENSION | 512 | 160 | 100 | 5.1x smaller | -37.5% |
| MESSAGE_CREATION | 1,253 | 406 | 272 | 4.6x smaller | -33.0% |
| LONG_LIST | 81,794 | 18,263 | 13,005 | 6.3x smaller | -28.8% |

Bytes allocated per parse:

| Case | ANTLR | Pratt before | Pratt after | Pratt vs ANTLR | Delta this CL |
| :--- | ---: | ---: | ---: | ---: | ---: |
| SMOKE_TEST | 12,256 | 3,512 | 2,040 | 6.0x smaller | -41.9% |
| CHAINED_ORS | 32,160 | 9,592 | 5,656 | 5.7x smaller | -41.0% |
| LIST_COMPREHENSION | 17,320 | 4,736 | 2,928 | 5.9x smaller | -38.2% |
| MESSAGE_CREATION | 43,128 | 12,416 | 8,696 | 5.0x smaller | -30.0% |
| LONG_LIST | 2,907,488 | 531,888 | 390,992 | 7.4x smaller | -26.5% |

Wall clock, mean of 3 caliper trial medians:

| Case | ANTLR | Pratt before | Pratt after | Pratt vs ANTLR | Delta this CL |
| :--- | ---: | ---: | ---: | ---: | ---: |
| SMOKE_TEST | 4,940 ns | 692 ns | 388 ns | 12.7x faster | -43.9% |
| CHAINED_ORS | 14,641 ns | 2,055 ns | 1,111 ns | 13.2x faster | -45.9% |
| LIST_COMPREHENSION | 7,514 ns | 1,231 ns | 797 ns | 9.4x faster | -35.3% |
| MESSAGE_CREATION | 20,979 ns | 3,583 ns | 2,524 ns | 8.3x faster | -29.6% |
| LONG_LIST | 1,616,631 ns | 142,764 ns | 117,823 ns | 13.7x faster | -17.5% |

PiperOrigin-RevId: 979625851
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant