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

Update datasketches version from 0.8.3 to 3.2.0 #3264

Closed
wants to merge 4 commits into from

Conversation

shoothzj
Copy link
Member

@shoothzj shoothzj commented May 9, 2022

Motivation

sketches-core 0.8.3 is released in 2016. Keep update. And it's now transfer to an apache project

Changes

update sketches-core to latest apache version

Performance test (using jmh)

Performance tests doesn't show any performance regression

0.8.3

Iteration   1: 22440898.750 ops/s
Iteration   2: 22446008.035 ops/s
Iteration   3: 22448625.120 ops/s
Iteration   4: 21927593.096 ops/s
Iteration   5: 21975718.907 ops/s

3.2.0

Iteration   1: 21653606.740 ops/s
Iteration   2: 21570153.077 ops/s
Iteration   3: 21117009.634 ops/s
Iteration   4: 22191429.289 ops/s
Iteration   5: 22220934.512 ops/s

@shoothzj shoothzj changed the title Update datasketches version Update datasketches version from 0.8.3 to 3.2.0 May 9, 2022
@shoothzj
Copy link
Member Author

Copy link
Contributor

@dlg99 dlg99 left a comment

Choose a reason for hiding this comment

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

LGTM

@merlimat
Copy link
Contributor

@shoothzj Can you also check the number of allocations? Using -prof gc.

I remember in earlier versions of datasketches (after 0.8.3) they had introduced some regression that added lot of heap allocations.

@merlimat
Copy link
Contributor

merlimat commented Jun 6, 2022

@shoothzj Just did a quick test. There are 4 bytes per each recorded sample with the new Datasketches. It would be good to understand why that is the case and if there's any way to configure DataSketches to avoid that.

DataSketches 0.8.3
Benchmark                                                           (statsProvider)   Mode  Cnt     Score     Error   Units
StatsLoggerBenchmark.recordLatency                                       Prometheus  thrpt    3    15.203 ±   2.787  ops/us
StatsLoggerBenchmark.recordLatency:·gc.alloc.rate                        Prometheus  thrpt    3     0.023 ±   0.368  MB/sec
StatsLoggerBenchmark.recordLatency:·gc.alloc.rate.norm                   Prometheus  thrpt    3     0.002 ±   0.027    B/op
StatsLoggerBenchmark.recordLatency:·gc.churn.G1_Eden_Space               Prometheus  thrpt    3     1.603 ±  50.660  MB/sec
StatsLoggerBenchmark.recordLatency:·gc.churn.G1_Eden_Space.norm          Prometheus  thrpt    3     0.116 ±   3.650    B/op
StatsLoggerBenchmark.recordLatency:·gc.count                             Prometheus  thrpt    3     1.000            counts
StatsLoggerBenchmark.recordLatency:·gc.time                              Prometheus  thrpt    3     2.000                ms
StatsLoggerBenchmark.recordLatency:·stack                                Prometheus  thrpt            NaN               ---


DataSketches 3.2.0
Benchmark                                                           (statsProvider)   Mode  Cnt     Score     Error   Units
StatsLoggerBenchmark.recordLatency                                       Prometheus  thrpt    3    15.965 ±   9.438  ops/us
StatsLoggerBenchmark.recordLatency:·gc.alloc.rate                        Prometheus  thrpt    3    63.314 ±  35.780  MB/sec
StatsLoggerBenchmark.recordLatency:·gc.alloc.rate.norm                   Prometheus  thrpt    3     4.377 ±   0.023    B/op
StatsLoggerBenchmark.recordLatency:·gc.churn.G1_Eden_Space               Prometheus  thrpt    3    57.793 ±   4.866  MB/sec
StatsLoggerBenchmark.recordLatency:·gc.churn.G1_Eden_Space.norm          Prometheus  thrpt    3     3.998 ±   2.456    B/op
StatsLoggerBenchmark.recordLatency:·gc.count                             Prometheus  thrpt    3     3.000            counts
StatsLoggerBenchmark.recordLatency:·gc.time                              Prometheus  thrpt    3     4.000                ms
StatsLoggerBenchmark.recordLatency:·stack                                Prometheus  thrpt            NaN               ---

@shoothzj
Copy link
Member Author

shoothzj commented Jun 7, 2022

@merlimat Sorry for my late reply, I also tested 27 days before. Also open an issue in apache/datasketches-java#398
If we are sensitive with performance, we need to keep version on 0.8.3?

Copy link
Contributor

@hangc0276 hangc0276 left a comment

Choose a reason for hiding this comment

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

We'd better keep the old version due to the performance issue.

@hangc0276 hangc0276 added the dependencies Pull requests that update a dependency file label Jul 25, 2022
@hangc0276 hangc0276 modified the milestones: 4.16.0, 4.17.0 Jul 25, 2022
@StevenLuMT
Copy link
Contributor

fix old workflow,please see #3455 for detail

@shoothzj shoothzj closed this Sep 9, 2022
@eolivelli
Copy link
Contributor

The issue has been closed
apache/datasketches-java#398

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants