Skip to content

Add MultiTagInsertionFilter #6184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

etki
Copy link
Contributor

@etki etki commented Apr 26, 2025

A tiny performance / clarity improvement. There's just one major potential issue with the MeterFilter.commonTags(): as it accepts Iterable, there's no guarantee that end user won't provide List, Set or another implementation that is not Tags, and the downstream code would then create a new Tags instance on every call, resulting in the heavyweight sorting. This PR makes sure that the concatenated instance is always Tags, plus carries some negligible good manners, not recreating an object if nothing has changed and providing a meaningful class name in the stack traces. As with other PRs, we can get a much more significant speedup if we provide an interface to create Tags from already sorted collections (see #6113), as here we can do the same process as in Tags.merge(), preallocating array of a necessary size and doing O(n) iterative merge.

The benchmarks used were the ones in #6174, OpenJDK 21.0.2, Intel N100 at fixed 2GHz. They simulate the worst case, when user provides a list and not a Tags instance. How to read the table: injected = number of tags to be inserted by the filter, supplied = mode of the number of tags in incoming Meter.Id. Incoming Meter.Id may have 0-64 tags, with 90% probability this number equals exactly mode, in other 10% cases it is evenly distributed across other values in 0-64 range. It included some manual editing, so there might be (hopefully not) editing mistakes.

injected supplied version ns/op instructions/op
0 0 main 33.160 ± 0.229 233.759
0 0 PR 3.049 ± 0.001 27.030
0 1 main 30.885 ± 0.066 243.405
0 1 PR 3.065 ± 0.092 27.031
0 2 main 53.590 ± 0.069 404.384
0 2 PR 3.067 ± 0.112 27.032
0 4 main 50.770 ± 0.085 398.816
0 4 PR 3.044 ± 0.001 27.034
0 8 main 51.077 ± 0.089 399.049
0 8 PR 3.041 ± 0.001 27.032
0 16 main 51.161 ± 0.092 398.622
0 16 PR 3.085 ± 0.179 27.049
0 32 main 26.964 ± 0.087 211.182
0 32 PR 3.042 ± 0.001 27.029
0 64 main 52.232 ± 0.189 401.197
0 64 PR 3.060 ± 0.031 27.053
1 0 main 122.293 ± 0.154 703.924
1 0 PR 49.965 ± 0.118 347.985
1 1 main 196.077 ± 0.258 1171.953
1 1 PR 115.836 ± 0.192 719.294
1 2 main 197.922 ± 0.432 1136.217
1 2 PR 131.757 ± 0.401 807.881
1 4 main 224.569 ± 0.327 1310.172
1 4 PR 138.881 ± 0.548 813.315
1 8 main 253.095 ± 0.295 1483.112
1 8 PR 179.739 ± 0.519 1081.986
1 16 main 304.351 ± 0.397 1823.919
1 16 PR 228.875 ± 0.389 1410.629
1 32 main 396.117 ± 2.037 2485.540
1 32 PR 316.573 ± 1.356 2041.127
1 64 main 551.046 ± 1.552 3705.438
1 64 PR 487.610 ± 1.934 3438.701
2 0 main 144.861 ± 0.220 881.858
2 0 PR 65.845 ± 0.177 447.011
2 1 main 226.178 ± 0.334 1342.564
2 1 PR 135.478 ± 0.329 792.401
2 2 main 260.228 ± 0.213 1486.790
2 2 PR 161.769 ± 0.296 912.836
2 4 main 279.380 ± 0.307 1577.057
2 4 PR 193.212 ± 0.321 1089.519
2 8 main 327.282 ± 0.557 1814.979
2 8 PR 241.633 ± 0.709 1327.601
2 16 main 408.249 ± 0.545 2269.491
2 16 PR 318.136 ± 0.642 1788.760
2 32 main 526.307 ± 0.554 3157.325
2 32 PR 441.163 ± 1.102 2592.231
2 64 main 766.677 ± 1.308 4993.405
2 64 PR 658.538 ± 1.695 4420.926
4 0 main 182.301 ± 0.245 1068.177
4 0 PR 77.630 ± 0.175 502.045
4 1 main 272.785 ± 0.396 1594.865
4 1 PR 161.470 ± 0.604 976.527
4 2 main 313.002 ± 0.439 1735.746
4 2 PR 195.466 ± 0.812 1044.121
4 4 main 392.977 ± 1.067 1992.682
4 4 PR 265.163 ± 0.678 1286.843
4 8 main 449.671 ± 0.706 2240.938
4 8 PR 346.768 ± 0.483 1633.115
4 16 main 549.557 ± 0.671 2767.403
4 16 PR 439.445 ± 0.513 2147.315
4 32 main 701.312 ± 0.647 3773.130
4 32 PR 593.615 ± 0.775 3248.515
4 64 main 960.017 ± 1.318 5842.836
4 64 PR 857.253 ± 1.779 5214.584
8 0 main 270.091 ± 0.372 1376.492
8 0 PR 98.874 ± 0.209 568.017
8 1 main 391.446 ± 0.758 2061.158
8 1 PR 209.299 ± 0.698 1159.608
8 2 main 442.482 ± 0.558 2258.830
8 2 PR 277.086 ± 0.725 1441.766
8 4 main 542.029 ± 0.635 2500.354
8 4 PR 369.562 ± 0.795 1677.081
8 8 main 699.364 ± 1.231 2976.732
8 8 PR 489.296 ± 1.543 2038.914
8 16 main 794.251 ± 0.965 3425.965
8 16 PR 626.974 ± 0.930 2619.581
8 32 main 983.273 ± 0.920 4673.640
8 32 PR 804.285 ± 0.678 3690.593
8 64 main 1254.053 ± 1.832 6834.259
8 64 PR 1066.830 ± 1.275 5944.678
16 0 main 445.825 ± 0.904 1949.532
16 0 PR 129.738 ± 0.200 651.577
16 1 main 641.263 ± 0.649 2981.935
16 1 PR 289.152 ± 0.436 1545.308
16 2 main 713.181 ± 0.690 3230.028
16 2 PR 372.448 ± 0.663 1842.125
16 4 main 830.807 ± 0.499 3617.535
16 4 PR 487.805 ± 0.843 2250.972
16 8 main 1004.185 ± 1.122 4012.767
16 8 PR 649.396 ± 0.764 2539.202
16 16 main 1237.963 ± 1.436 4615.991
16 16 PR 895.591 ± 1.937 3245.092
16 32 main 1487.600 ± 1.168 5901.954
16 32 PR 1145.585 ± 5.210 4403.641
16 64 main 1756.124 ± 2.213 7953.433
16 64 PR 1396.981 ± 5.869 6502.333
32 0 main 839.912 ± 0.733 3146.404
32 0 PR 165.206 ± 0.337 714.254
32 1 main 1103.111 ± 1.202 4841.557
32 1 PR 413.303 ± 0.512 2277.516
32 2 main 1222.819 ± 1.091 5384.683
32 2 PR 530.356 ± 0.653 2812.296
32 4 main 1364.309 ± 1.599 6013.481
32 4 PR 664.749 ± 0.785 3339.794
32 8 main 1548.493 ± 1.255 6323.487
32 8 PR 865.435 ± 0.966 3810.349
32 16 main 1851.388 ± 1.451 6880.047
32 16 PR 1159.547 ± 5.038 4357.667
32 32 main 2253.046 ± 8.181 7934.419
32 32 PR 1534.134 ± 3.198 5265.580
32 64 main 2362.974 ± 2.457 9340.686
32 64 PR 1705.116 ± 7.068 6664.152
64 0 main 874.147 ± 1.409 5728.756
64 0 PR 166.424 ± 0.374 871.560
64 1 main 1270.810 ± 2.079 8656.661
64 1 PR 566.643 ± 0.696 3771.090
64 2 main 1469.561 ± 2.525 9697.014
64 2 PR 742.514 ± 1.207 4789.128
64 4 main 1666.623 ± 2.374 10802.580
64 4 PR 916.976 ± 1.348 5675.273
64 8 main 1859.909 ± 2.711 10988.170
64 8 PR 1118.062 ± 1.540 6049.757
64 16 main 2219.444 ± 2.526 11789.233
64 16 PR 1404.797 ± 2.394 6715.599
64 32 main 2439.326 ± 3.140 11717.007
64 32 PR 1683.831 ± 7.463 6683.118
64 64 main 1941.294 ± 2.460 12158.768
64 64 PR 1191.606 ± 1.200 7225.046

return NoOpFilter.create();
}

return MultiTagInsertionFilter.of(collected);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There would be some sense in creating a OneTwoInsertionFilter IF we'd have the API from #6113. This is the case in my company, we insert only a single tag, and getting rid of collection abstractions could have provided extra boost. However, since any addition here triggers the sorting process, there's no sense in creating a special implementation for the most frequent case.

public Meter.Id map(Meter.Id id) {
Iterable<Tag> original = id.getTagsAsIterable();
Tags replacement = original == Tags.empty() ? injection : Tags.concat(injection, original);
return id.replaceTags(replacement);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not doing replacement == original check as it's impossible - even if all the injected tags would get overridden, this would not result in the same instance being returned. Calling .equals() is redundant because it involves iterative string comparison, it will outweigh any potential savings on that single allocation.

@jonatan-ivanov jonatan-ivanov added the performance Issues related to general performance label Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Issues related to general performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants