Skip to content
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

Skip redundant tag deduplication #3902

Conversation

Stephan202
Copy link
Contributor

Both KeyValues and Tags unconditionally maintain the invariant that
they wrap distinct key-value pairs.

@Stephan202 Stephan202 requested a review from a team as a code owner October 3, 2024 22:12
@Stephan202 Stephan202 force-pushed the sschroevers/skip-redundant-tag-deduplication branch from 91f804c to a9062ba Compare October 3, 2024 22:21
Copy link
Contributor Author

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

This PR relates to #3900 and #3901, in that all attempt to reduce the cost of instrumenting reactive streams.

@@ -13,15 +13,18 @@ configurations {
dependencies {
// Use the baseline to avoid using new APIs in the benchmarks
compileOnly libs.reactor.perfBaseline.core
compileOnly libs.reactor.perfBaseline.coreMicrometer
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not too familiar with Gradle, so happy to receive feedback if I should have modified this file (or gradle/libs.versions.toml below) in some other way.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, this line together with the versions catalog are ok. I'll comment below what needs to be adjusted.

Comment on lines +40 to +61
@SuppressWarnings("unused")
@Benchmark
public Tags measureThroughput() {
return MicrometerMeterListenerConfiguration.resolveTags(publisher, Tags.of("k", "v"));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Benchmark results:

Before

Benchmark                                                                                      (distinctTagCount)  (totalTagCount)  Mode  Cnt     Score     Error   Units
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      1                1  avgt    5   159.226 ±   8.332   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        1                1  avgt    5  7331.787 ± 377.812  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   1                1  avgt    5  1224.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             1                1  avgt    5   100.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              1                1  avgt    5   161.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      1                2  avgt    5   217.163 ±   3.158   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        1                2  avgt    5  5972.192 ±  87.004  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   1                2  avgt    5  1360.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             1                2  avgt    5    81.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              1                2  avgt    5   115.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      1                5  avgt    5   237.565 ±   7.034   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        1                5  avgt    5  4945.661 ± 145.100  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   1                5  avgt    5  1232.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             1                5  avgt    5    67.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              1                5  avgt    5    96.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      1               10  avgt    5   286.132 ±  22.135   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        1               10  avgt    5  4107.322 ± 309.521  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   1               10  avgt    5  1232.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             1               10  avgt    5    56.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              1               10  avgt    5    83.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      2                1  avgt    5   142.298 ±   3.436   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        2                1  avgt    5  8202.963 ± 198.838  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   2                1  avgt    5  1224.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             2                1  avgt    5   112.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              2                1  avgt    5   166.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      2                2  avgt    5   254.355 ±   7.278   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        2                2  avgt    5  5369.084 ± 152.575  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   2                2  avgt    5  1432.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             2                2  avgt    5    73.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              2                2  avgt    5   107.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      2                5  avgt    5   367.433 ±   5.074   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        2                5  avgt    5  3384.417 ±  46.660  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   2                5  avgt    5  1304.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             2                5  avgt    5    46.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              2                5  avgt    5    70.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      2               10  avgt    5   343.450 ±   0.914   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        2               10  avgt    5  3976.036 ±  10.634  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   2               10  avgt    5  1432.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             2               10  avgt    5    54.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              2               10  avgt    5    84.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      5                1  avgt    5   150.793 ±   6.996   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        5                1  avgt    5  7741.417 ± 353.730  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   5                1  avgt    5  1224.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             5                1  avgt    5   105.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              5                1  avgt    5   133.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      5                2  avgt    5   321.504 ±   5.429   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        5                2  avgt    5  3867.861 ±  64.989  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   5                2  avgt    5  1304.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             5                2  avgt    5    52.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              5                2  avgt    5    75.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      5                5  avgt    5   488.914 ±   8.663   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        5                5  avgt    5  3214.443 ±  57.356  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   5                5  avgt    5  1648.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             5                5  avgt    5    43.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              5                5  avgt    5    61.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      5               10  avgt    5   527.945 ±  10.297   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        5               10  avgt    5  2861.202 ±  55.557  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   5               10  avgt    5  1584.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             5               10  avgt    5    39.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              5               10  avgt    5    61.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                     10                1  avgt    5   153.768 ±   6.539   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                       10                1  avgt    5  7591.569 ± 319.761  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                  10                1  avgt    5  1224.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                            10                1  avgt    5   104.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                             10                1  avgt    5   140.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                     10                2  avgt    5   349.661 ±  11.149   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                       10                2  avgt    5  3905.636 ± 123.250  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                  10                2  avgt    5  1432.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                            10                2  avgt    5    53.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                             10                2  avgt    5    76.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                     10                5  avgt    5   453.534 ±  28.053   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                       10                5  avgt    5  3465.851 ± 210.809  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                  10                5  avgt    5  1648.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                            10                5  avgt    5    47.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                             10                5  avgt    5    70.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                     10               10  avgt    5   647.491 ±  33.038   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                       10               10  avgt    5  2957.819 ± 148.431  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                  10               10  avgt    5  2008.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                            10               10  avgt    5    40.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                             10               10  avgt    5    58.000                ms

After

Benchmark                                                                                      (distinctTagCount)  (totalTagCount)  Mode  Cnt     Score     Error   Units
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      1                1  avgt    5   122.536 ±   0.860   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        1                1  avgt    5  6599.459 ±  46.318  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   1                1  avgt    5   848.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             1                1  avgt    5    89.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              1                1  avgt    5   125.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      1                2  avgt    5   147.859 ±   6.440   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        1                2  avgt    5  5521.359 ± 237.145  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   1                2  avgt    5   856.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             1                2  avgt    5    75.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              1                2  avgt    5   113.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      1                5  avgt    5   221.202 ±  17.544   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        1                5  avgt    5  4381.561 ± 338.380  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   1                5  avgt    5  1016.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             1                5  avgt    5    60.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              1                5  avgt    5    91.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      1               10  avgt    5   316.081 ±   4.174   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        1               10  avgt    5  3354.992 ±  44.047  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   1               10  avgt    5  1112.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             1               10  avgt    5    46.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              1               10  avgt    5    62.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      2                1  avgt    5   120.735 ±   2.683   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        2                1  avgt    5  6445.464 ± 142.300  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   2                1  avgt    5   816.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             2                1  avgt    5    87.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              2                1  avgt    5   121.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      2                2  avgt    5   142.788 ±   3.936   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        2                2  avgt    5  5717.046 ± 156.415  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   2                2  avgt    5   856.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             2                2  avgt    5    78.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              2                2  avgt    5   112.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      2                5  avgt    5   337.520 ±   4.360   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        2                5  avgt    5  2780.195 ±  35.886  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   2                5  avgt    5   984.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             2                5  avgt    5    38.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              2                5  avgt    5    61.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      2               10  avgt    5   586.248 ±  15.748   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        2               10  avgt    5  1913.004 ±  50.940  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   2               10  avgt    5  1176.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             2               10  avgt    5    26.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              2               10  avgt    5    38.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      5                1  avgt    5   122.197 ±   5.106   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        5                1  avgt    5  6618.401 ± 272.695  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   5                1  avgt    5   848.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             5                1  avgt    5    90.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              5                1  avgt    5   127.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      5                2  avgt    5   253.426 ±  13.879   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        5                2  avgt    5  3342.005 ± 179.684  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   5                2  avgt    5   888.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             5                2  avgt    5    46.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              5                2  avgt    5    66.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      5                5  avgt    5   348.439 ±  11.319   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        5                5  avgt    5  2605.632 ±  83.665  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   5                5  avgt    5   952.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             5                5  avgt    5    35.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              5                5  avgt    5    52.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                      5               10  avgt    5  1080.822 ±  21.818   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                        5               10  avgt    5  1009.391 ±  20.206  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                   5               10  avgt    5  1144.002 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                             5               10  avgt    5    13.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                              5               10  avgt    5    23.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                     10                1  avgt    5   114.838 ±   5.161   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                       10                1  avgt    5  6776.790 ± 300.038  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                  10                1  avgt    5   816.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                            10                1  avgt    5    92.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                             10                1  avgt    5   138.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                     10                2  avgt    5   254.114 ±   6.417   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                       10                2  avgt    5  3452.612 ±  86.498  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                  10                2  avgt    5   920.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                            10                2  avgt    5    47.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                             10                2  avgt    5    70.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                     10                5  avgt    5   314.048 ±   1.360   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                       10                5  avgt    5  3085.122 ±  13.391  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                  10                5  avgt    5  1016.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                            10                5  avgt    5    42.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                             10                5  avgt    5    61.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                                     10               10  avgt    5   453.549 ±  19.844   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate                       10               10  avgt    5  2472.887 ± 107.505  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm                  10               10  avgt    5  1176.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                            10               10  avgt    5    34.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                             10               10  avgt    5    52.000                ms

This shows that the new code is faster (up to 55%) and allocates less memory (up to 50%) in the common case, where few or no keys are duplicated. In cases where keys are heavily duplicated (distinctTagCount < 10, totalTagCount = 10), the new logic is slower but (a) this is not a scenerio one would expect to observe in practice and (b) users should generally be able to avoid duplicates, while they do not have control over the deduplication implementation.

Copy link
Member

Choose a reason for hiding this comment

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

micrometer-metrics/micrometer#4959 was just merged yesterday. I'm really curious how that performs together with your changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for sharing! Nice timing, and thanks for pushing that @mstyura 💪

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I re-ran the before- and after-benchmarks against the latest Micrometer snapshot.

Before

Benchmark                                                                                      (testCase)  Mode  Cnt     Score     Error   Units
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                            1|1  avgt    5   232.700 ±  12.088   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate              1|1  avgt    5  5377.355 ± 275.858  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm         1|1  avgt    5  1312.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                   1|1  avgt    5    73.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                    1|1  avgt    5    88.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                            1|2  avgt    5   253.188 ±   5.480   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate              1|2  avgt    5  4971.838 ± 106.974  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm         1|2  avgt    5  1320.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                   1|2  avgt    5    67.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                    1|2  avgt    5    87.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                            1|5  avgt    5   292.085 ±  19.269   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate              1|5  avgt    5  4519.590 ± 292.017  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm         1|5  avgt    5  1384.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                   1|5  avgt    5    61.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                    1|5  avgt    5    66.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                           1|10  avgt    5   303.817 ±  16.276   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate             1|10  avgt    5  4143.854 ± 218.000  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm        1|10  avgt    5  1320.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                  1|10  avgt    5    57.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                   1|10  avgt    5    73.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                            2|2  avgt    5   274.764 ±  17.355   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate              2|2  avgt    5  5054.366 ± 312.503  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm         2|2  avgt    5  1456.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                   2|2  avgt    5    68.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                    2|2  avgt    5    96.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                            2|5  avgt    5   384.112 ±  17.198   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate              2|5  avgt    5  3615.106 ± 159.562  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm         2|5  avgt    5  1456.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                   2|5  avgt    5    49.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                    2|5  avgt    5    59.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                           2|10  avgt    5   320.729 ±  11.327   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate             2|10  avgt    5  4139.081 ± 144.604  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm        2|10  avgt    5  1392.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                  2|10  avgt    5    56.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                   2|10  avgt    5    63.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                            5|5  avgt    5   415.296 ±  21.861   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate              5|5  avgt    5  3692.877 ± 191.056  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm         5|5  avgt    5  1608.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                   5|5  avgt    5    50.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                    5|5  avgt    5    56.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                           5|10  avgt    5   458.328 ±  14.505   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate             5|10  avgt    5  3345.821 ± 104.779  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm        5|10  avgt    5  1608.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                  5|10  avgt    5    46.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                   5|10  avgt    5    57.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                          10|10  avgt    5   606.856 ±  41.112   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate            10|10  avgt    5  3093.244 ± 204.835  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm       10|10  avgt    5  1968.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                 10|10  avgt    5    43.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                  10|10  avgt    5    47.000                ms

After

Benchmark                                                                                      (testCase)  Mode  Cnt     Score     Error   Units
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                            1|1  avgt    5   139.095 ±   9.897   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate              1|1  avgt    5  6089.635 ± 423.860  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm         1|1  avgt    5   888.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                   1|1  avgt    5    82.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                    1|1  avgt    5   103.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                            1|2  avgt    5   170.519 ±   2.546   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate              1|2  avgt    5  5324.073 ±  79.438  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm         1|2  avgt    5   952.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                   1|2  avgt    5    73.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                    1|2  avgt    5    81.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                            1|5  avgt    5   230.257 ±   2.628   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate              1|5  avgt    5  4307.261 ±  49.140  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm         1|5  avgt    5  1040.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                   1|5  avgt    5    58.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                    1|5  avgt    5    70.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                           1|10  avgt    5   278.842 ±   9.763   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate             1|10  avgt    5  3912.650 ± 135.778  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm        1|10  avgt    5  1144.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                  1|10  avgt    5    53.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                   1|10  avgt    5    66.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                            2|2  avgt    5   157.001 ±   6.063   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate              2|2  avgt    5  5831.547 ± 222.480  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm         2|2  avgt    5   960.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                   2|2  avgt    5    79.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                    2|2  avgt    5    86.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                            2|5  avgt    5   298.583 ±   9.338   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate              2|5  avgt    5  3245.048 ± 100.476  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm         2|5  avgt    5  1016.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                   2|5  avgt    5    44.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                    2|5  avgt    5    50.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                           2|10  avgt    5   396.633 ±  14.369   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate             2|10  avgt    5  2769.929 ±  99.182  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm        2|10  avgt    5  1152.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                  2|10  avgt    5    38.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                   2|10  avgt    5    39.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                            5|5  avgt    5   293.354 ±  15.722   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate              5|5  avgt    5  3329.246 ± 175.336  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm         5|5  avgt    5  1024.000 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                   5|5  avgt    5    46.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                    5|5  avgt    5    61.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                           5|10  avgt    5   452.498 ±  28.634   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate             5|10  avgt    5  2512.614 ± 155.733  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm        5|10  avgt    5  1192.001 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                  5|10  avgt    5    35.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                   5|10  avgt    5    42.000                ms
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput                          10|10  avgt    5   791.217 ±  16.072   ns/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate            10|10  avgt    5  1427.047 ±  28.815  MB/sec
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.alloc.rate.norm       10|10  avgt    5  1184.002 ±   0.001    B/op
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.count                 10|10  avgt    5    19.000            counts
MicrometerMeterListenerConfigurationResolveTagsBenchmark.measureThroughput:gc.time                  10|10  avgt    5    23.000                ms

We see that in all cases less memory is allocated. The 2|10 and 10|10 cases show a small performance degradation, but in all other cases the new code is faster or on-par. Since (in my experience) in most cases there will be fewer than 10 tags, I'd say this is an over-all performance improvement.

@@ -295,9 +295,8 @@ static Tags resolveTags(Publisher<?> source, Tags tags) {
Scannable scannable = Scannable.from(source);

if (scannable.isScanAvailable()) {
List<Tag> discoveredTags = scannable.tagsDeduplicated()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The method Scannable#tagsDeduplicated() is now unused, but I didn't drop it, as it's part of a public interface.

Copy link
Member

Choose a reason for hiding this comment

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

I don't suspect tagsDuplicated() is used outside of our codebase. In any case we should be able to deprecate it with the explanation of the observation you made that typical usages of the stream with duplicates do deduplication so it's unnecessary to de-duplicate them by design. The method is quite straightforward so it can be replicated in case anyone needs it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deprecating the method SGTM; indeed a GitHub search does not seem to turn up additional usages. I've proposed a @deprecated sentence that hopefully explains the situation, without claiming that no caller would find the method useful.

Comment on lines 95 to 101
if (scannable.isScanAvailable()) {
List<Tag> discoveredTags = scannable.tagsDeduplicated()
.entrySet().stream()
.map(e -> Tag.of(e.getKey(), e.getValue()))
List<Tag> discoveredTags = scannable.tags()
.map(t -> Tag.of(t.getT1(), t.getT2()))
.collect(Collectors.toList());
return tags.and(discoveredTags);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code snippet occurred in three places; all three are updated in an identical manner. In each case the code is covered by a test that validates the desired deduplication logic ("last one wins"). (For brevity the new benchmark covers one of the two non-deprecated classes.)

Copy link
Member

Choose a reason for hiding this comment

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

That's a very important observation actually and I believe it deserves to be documented (not in javadocs, but just a line comment).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check, I added a comment about the "last one wins" strategy. Let me know if you instead meant that the code snippets should reference each other and/or all point to the same benchmark class.

Copy link
Member

@chemicL chemicL left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. It looks very promising. I left a few comments.

Comment on lines 95 to 101
if (scannable.isScanAvailable()) {
List<Tag> discoveredTags = scannable.tagsDeduplicated()
.entrySet().stream()
.map(e -> Tag.of(e.getKey(), e.getValue()))
List<Tag> discoveredTags = scannable.tags()
.map(t -> Tag.of(t.getT1(), t.getT2()))
.collect(Collectors.toList());
return tags.and(discoveredTags);
}
Copy link
Member

Choose a reason for hiding this comment

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

That's a very important observation actually and I believe it deserves to be documented (not in javadocs, but just a line comment).

@@ -295,9 +295,8 @@ static Tags resolveTags(Publisher<?> source, Tags tags) {
Scannable scannable = Scannable.from(source);

if (scannable.isScanAvailable()) {
List<Tag> discoveredTags = scannable.tagsDeduplicated()
Copy link
Member

Choose a reason for hiding this comment

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

I don't suspect tagsDuplicated() is used outside of our codebase. In any case we should be able to deprecate it with the explanation of the observation you made that typical usages of the stream with duplicates do deduplication so it's unnecessary to de-duplicate them by design. The method is quite straightforward so it can be replicated in case anyone needs it.

Comment on lines 43 to 47
@Param({"1", "2", "5", "10"})
private int distinctTagCount;

@Param({"1", "2", "5", "10"})
private int totalTagCount;
Copy link
Member

Choose a reason for hiding this comment

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

Consider that this results in an outer product, which means a full matrix of combinations while you only need half of it. The runs for distinct = 10, 5, 2, 1 with total = 1exercise the same dataset. The same for distinct = 10, 5, 2, total = 2, and so on. Perhaps a String that you can split in @Setup would be useful, e.g.

@Param({"1|1", "1|2", "1|5", "1|10", "2|2", "2|5", ...})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice idea; applied!

Comment on lines +40 to +61
@SuppressWarnings("unused")
@Benchmark
public Tags measureThroughput() {
return MicrometerMeterListenerConfiguration.resolveTags(publisher, Tags.of("k", "v"));
}
Copy link
Member

Choose a reason for hiding this comment

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

micrometer-metrics/micrometer#4959 was just merged yesterday. I'm really curious how that performs together with your changes.

@@ -9,6 +9,7 @@
# Baselines, should be updated on every release
baseline-core-api = "3.6.10"
baselinePerfCore = "3.6.10"
baselinePerfCoreMicrometer = "1.1.9"
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason why you chose 1.1.9 instead of 1.1.10?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hah, I took the most recent version from my local Maven repo; should have checked Maven Central instead, of course.

@chemicL chemicL added status/need-user-input This needs user input to proceed area/performance This belongs to the performance theme labels Oct 4, 2024
Copy link
Contributor Author

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

Thanks for the review @chemicL! I added a commit addressing the feedback.

Comment on lines 43 to 47
@Param({"1", "2", "5", "10"})
private int distinctTagCount;

@Param({"1", "2", "5", "10"})
private int totalTagCount;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice idea; applied!

Comment on lines +40 to +61
@SuppressWarnings("unused")
@Benchmark
public Tags measureThroughput() {
return MicrometerMeterListenerConfiguration.resolveTags(publisher, Tags.of("k", "v"));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for sharing! Nice timing, and thanks for pushing that @mstyura 💪

@@ -9,6 +9,7 @@
# Baselines, should be updated on every release
baseline-core-api = "3.6.10"
baselinePerfCore = "3.6.10"
baselinePerfCoreMicrometer = "1.1.9"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hah, I took the most recent version from my local Maven repo; should have checked Maven Central instead, of course.

Comment on lines 95 to 101
if (scannable.isScanAvailable()) {
List<Tag> discoveredTags = scannable.tagsDeduplicated()
.entrySet().stream()
.map(e -> Tag.of(e.getKey(), e.getValue()))
List<Tag> discoveredTags = scannable.tags()
.map(t -> Tag.of(t.getT1(), t.getT2()))
.collect(Collectors.toList());
return tags.and(discoveredTags);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check, I added a comment about the "last one wins" strategy. Let me know if you instead meant that the code snippets should reference each other and/or all point to the same benchmark class.

@@ -295,9 +295,8 @@ static Tags resolveTags(Publisher<?> source, Tags tags) {
Scannable scannable = Scannable.from(source);

if (scannable.isScanAvailable()) {
List<Tag> discoveredTags = scannable.tagsDeduplicated()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deprecating the method SGTM; indeed a GitHub search does not seem to turn up additional usages. I've proposed a @deprecated sentence that hopefully explains the situation, without claiming that no caller would find the method useful.

Copy link
Member

@chemicL chemicL left a comment

Choose a reason for hiding this comment

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

Please see my comments regardng the benchmark setup and merge with latest main to get the versions aligned. I think it's reasonable to merge this proposal after the changes, thanks!

@@ -13,15 +13,18 @@ configurations {
dependencies {
// Use the baseline to avoid using new APIs in the benchmarks
compileOnly libs.reactor.perfBaseline.core
compileOnly libs.reactor.perfBaseline.coreMicrometer
Copy link
Member

Choose a reason for hiding this comment

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

Yep, this line together with the versions catalog are ok. I'll comment below what needs to be adjusted.

annotationProcessor "org.openjdk.jmh:jmh-generator-annprocess:$jmhVersion"

current project(':reactor-core')
current project(':reactor-core-micrometer')
Copy link
Member

Choose a reason for hiding this comment

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

Please also modify the baseline source set and add the reactor-core-micrometer module with appropriate version. The idea here is that we can have a comparison of performance in the previously released version against the current version. That's why there's a "current" and a "baseline" source set.

@Stephan202 Stephan202 force-pushed the sschroevers/skip-redundant-tag-deduplication branch from da9c82b to 113d13b Compare October 19, 2024 12:53
Copy link
Contributor Author

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

I've rebased the branch and applied the requested change. Tnx for the review @chemicL!

@@ -9,6 +9,7 @@
# Baselines, should be updated on every release
baseline-core-api = "3.6.10"
baselinePerfCore = "3.6.10"
baselinePerfCoreMicrometer = "1.1.10"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By now 1.1.11 is out; will update.

@Stephan202
Copy link
Contributor Author

Hey @chemicL! Anything else I should do to move this PR forward? Thanks for considering 🙏

@chemicL
Copy link
Member

chemicL commented Nov 21, 2024

@Stephan202 please merge in the latest main, update to the latest versions and we can merge.

Both `KeyValues` and `Tags` unconditionally maintain the invariant that
they wrap distinct key-value pairs.
@Stephan202 Stephan202 force-pushed the sschroevers/skip-redundant-tag-deduplication branch from 113d13b to 7df3f70 Compare November 21, 2024 17:04
@Stephan202
Copy link
Contributor Author

Cool! I rebased and added a commit to bump the new dependency once more. Pre-push diff-of-diff for validation purposes:

$ diff -u <(git diff main...origin/sschroevers/skip-redundant-tag-deduplication) <(git diff main...sschroevers/skip-redundant-tag-deduplication)
--- /dev/fd/63	2024-11-21 18:01:47.805553471 +0100
+++ /dev/fd/62	2024-11-21 18:01:47.806553477 +0100
@@ -104,14 +104,14 @@
 +	}
 +}
 diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
-index a4139cb8e..a689711d8 100644
+index 20d092160..70f9fba45 100644
 --- a/gradle/libs.versions.toml
 +++ b/gradle/libs.versions.toml
 @@ -9,6 +9,7 @@
  # Baselines, should be updated on every release
- baseline-core-api = "3.6.11"
- baselinePerfCore = "3.6.11"
-+baselinePerfCoreMicrometer = "1.1.11"
+ baseline-core-api = "3.7.0"
+ baselinePerfCore = "3.7.0"
++baselinePerfCoreMicrometer = "1.2.0"
  baselinePerfExtra = "3.5.2"
  
  # Other shared versions

Locally I do see one test failure, but it also happens on main, so let's see what CI thinks 👀

ApplyingByteBuddyPluginGradleTest > applyingByteBuddyPluginDuringGradleBuild() FAILED
    org.opentest4j.AssertionFailedError: expected: <SUCCESS> but was: <UP_TO_DATE>
        at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
        at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
        at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
        at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
        at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
        at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
        at reactor.tools.agent.ApplyingByteBuddyPluginGradleTest.applyingByteBuddyPluginDuringGradleBuild(ApplyingByteBuddyPluginGradleTest.java:59)

@chemicL chemicL removed the status/need-user-input This needs user input to proceed label Nov 25, 2024
@chemicL
Copy link
Member

chemicL commented Nov 25, 2024

I am unable to trigger the above test failure nor did CI observe it. Seems unrelated nevertheless.

@chemicL chemicL merged commit 7cc701c into reactor:main Nov 25, 2024
7 checks passed
@chemicL
Copy link
Member

chemicL commented Nov 25, 2024

@Stephan202 thank you for your contribution, it has now been merged 🚢

@Stephan202 Stephan202 deleted the sschroevers/skip-redundant-tag-deduplication branch November 25, 2024 16:28
@violetagg violetagg added the type/enhancement A general enhancement label Dec 10, 2024
@violetagg violetagg added this to the 3.7.1 milestone Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/observability area/performance This belongs to the performance theme type/enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants