[XS] Change in dart/sdk[main]: [analyzer] BinaryWriter has a smaller buffer
0 views
Skip to first unread message
Jens Johansen (Gerrit)
Oct 31, 2025, 6:54:43 AM (5 days ago) Oct 31
to Johnni Winther, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Johnni Winther
Jens Johansen added 1 comment![]( "Open in Gerrit")
File pkg/analyzer/lib/src/binary/binary_writer.dart
Line 41, Patchset 1:
return _builder.takeBytes();Jens Johansen . resolved
With `BytesBuilder` now being a copying one this returns a view of something thats possible quite a bit larger. `_builder.toBytes()` would do a new copy making it not bigger, but which is another copy which makes it slower (at about 3%!).
So I'll undo this change and do the simple one instead.
Related details
Attention is currently required from:
- Johnni Winther
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Johnni Winther (Gerrit)
Oct 31, 2025, 7:04:37 AM (5 days ago) Oct 31
to Jens Johansen, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Jens Johansen
Johnni Winther voted and added 2 comments![]( "Open in Gerrit")
Votes added by Johnni Winther
| Code-Review | +1 |
2 comments
Commit Message
Line 11, Patchset 3 (Latest):
to compile the analysis server) this results in allocating ~2.7GB (withJohnni Winther . unresolved
How much does it allocate after this change?
Line 12, Patchset 3 (Latest):
`takeBytes` returning a total of <54MB in this case).Johnni Winther . unresolved
How many allocations of `BinaryWriter` do we have? / What is that average and variance of the size returned from each `BinaryWriter`?
Related details
Attention is currently required from:
- Jens Johansen
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jens Johansen (Gerrit)
Oct 31, 2025, 7:42:03 AM (5 days ago) Oct 31
to Johnni Winther, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Jens Johansen added 2 comments![]( "Open in Gerrit")
Commit Message
Line 11, Patchset 3 (Latest):
to compile the analysis server) this results in allocating ~2.7GB (withJohnni Winther . resolved
How much does it allocate after this change?
Jens Johansen
(answered below).
Line 12, Patchset 3 (Latest):
`takeBytes` returning a total of <54MB in this case).Johnni Winther . resolved
How many allocations of `BinaryWriter` do we have? / What is that average and variance of the size returned from each `BinaryWriter`?
Jens Johansen
For this particular one (at least I think I ran it on the same thing again) we have 22272 allocations of `BinaryWriter` which gives 2.71875 GB which fits with what I wrote here.
Equivalently there's 22272 `takeBytes` calls with the median returned bytes being 306 bytes. 20939 of the `takeBytes` calls (~94%) return 5119 or less bytes (and would thus avoid a new allocation with the smaller 5KB buffer).
The sum of the returned bytes from `takeBytes` is 56181728 bytes aka ~53.58MB.
```
N Min Max Median Avg Stddev
x 22272 4 3540467 306 2522.5273 37821.24
```
In total - with a 128KB buffer - it allocates 22353 such buffers (81 more than the number of `BinaryWriter`s because there are a few that are actually larger than this buffer size).
With the change to a 5KB buffer it does 26540 allocations of this buffer (i.e. 4268 more than the number of `BinaryWriter`s) for a total of 135884800 bytes aka ~130MB.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jens Johansen (Gerrit)
Oct 31, 2025, 7:44:36 AM (5 days ago) Oct 31
to Johnni Winther, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Jens Johansen voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Commit Queue (Gerrit)
Oct 31, 2025, 7:44:45 AM (5 days ago) Oct 31
to Jens Johansen, Johnni Winther, dart-analys...@google.com, rev...@dartlang.org
Commit Queue submitted the change![]( "Open in Gerrit")
Unreviewed changes
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Commit message:
[analyzer] BinaryWriter has a smaller buffer
Previously whenever a BinaryWriter was created it immediately created a
128 kb Uint8List. When compiling analysis server et al (all sources used
to compile the analysis server) this results in allocating ~2.7GB (with
`takeBytes` returning a total of <54MB in this case).
This CL reduces this buffer to 5KB instead, reducing the overhead on GC.
With a 5KB buffer (about 94% `takeBytes` calls return less than 5KB)
we instead allocate about 130MB in this case.
It being mostly a GC pressure change (and that it will change up when
GC happens) benchmarking it isn't great, nevertheless this is what I
got:
Analyzing CFE et al (without dart2wasm stuff):
```
msec task-clock:u: -1.9193% +/- 0.5797% (-166.76 +/- 50.37) (8688.58 -> 8521.82)
page-faults:u: -7.8067% +/- 0.0517% (-12297.80 +/- 81.48) (157529.64 -> 145231.84)
cycles:u: -1.6454% +/- 0.6021% (-600019148.06 +/- 219543431.36) (36465937621.72 -> 35865918473.66)
instructions:u: -1.1447% +/- 0.4261% (-498854632.50 +/- 185671028.34) (43579529159.92 -> 43080674527.42)
branch-misses:u: -2.8986% +/- 1.9360% (-4171049.68 +/- 2785886.58) (143898017.08 -> 139726967.40)
seconds time elapsed: -1.9201% +/- 0.5796% (-0.17 +/- 0.05) (8.69 -> 8.53)
seconds user: -1.7259% +/- 0.6256% (-0.14 +/- 0.05) (8.37 -> 8.23)
seconds sys: -7.0216% +/- 4.0546% (-0.02 +/- 0.01) (0.32 -> 0.29)
Comparing GC data:
Scavenge( new space) goes from 176 to 138
MarkSweep( old space) goes from 1 to 2
Evacuate(store buffer) goes from 1 to 0
Notice combined GC time goes from 2811 ms to 2959 ms (notice only 1 run each).
```
Analysis server et al:
```
msec task-clock:u: -2.6522% +/- 0.5847% (-467.52 +/- 103.07) (17627.23 -> 17159.71)
page-faults:u: -3.5245% +/- 0.0502% (-10456.22 +/- 148.92) (296669.38 -> 286213.16)
cycles:u: -2.5953% +/- 0.5919% (-1921850003.72 +/- 438308899.83) (74050606844.82 -> 72128756841.10)
instructions:u: -1.8325% +/- 0.0006% (-1578705053.84 +/- 483572.09) (86149571116.38 -> 84570866062.54)
branch-misses:u: -4.0379% +/- 2.0070% (-12275829.02 +/- 6101639.40) (304013449.34 -> 291737620.32)
seconds time elapsed: -2.6510% +/- 0.5845% (-0.47 +/- 0.10) (17.64 -> 17.17)
seconds user: -2.5518% +/- 0.6162% (-0.43 +/- 0.10) (17.00 -> 16.56)
seconds sys: -5.3499% +/- 2.5859% (-0.03 +/- 0.02) (0.63 -> 0.60)
Comparing GC data:
Scavenge( new space) goes from 363 to 273
MarkSweep( promotion) goes from 21 to 20
Evacuate(store buffer) goes from 1 to 0
Notice combined GC time goes from 5415 ms to 5542 ms (notice only 1 run each).
```
Change-Id: I78b1a9a1c882ad486eff97b3aee1ed5f29235891
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/458560
Commit-Queue: Jens Johansen <je...@google.com>
Reviewed-by: Johnni Winther <johnni...@google.com>
Files:
- M pkg/analyzer/lib/src/binary/binary_writer.dart
Change size: XS
Delta: 1 file changed, 1 insertion(+), 1 deletion(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Johnni Winther
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages