Skip to content

Introduce cache of suppliers and remove usage of Objects.hash to speed up #792

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

Merged
merged 3 commits into from
Apr 29, 2023

Conversation

snuyanzin
Copy link
Collaborator

The problem with Objects.hash is that it generates an array each time it is called.

Suppliers could be cached to avoid parsing of expressions each time

it gives speed up for a wide range of methods up to 50 %

expressions

before
Benchmark                        Mode  Cnt     Score     Error   Units
DatafakerExpressions.bothify    thrpt   10  5089.609 ± 112.084  ops/ms
DatafakerExpressions.letterify  thrpt   10  5706.392 ± 326.134  ops/ms
DatafakerExpressions.numerify   thrpt   10  6027.549 ±  87.064  ops/ms
DatafakerExpressions.regexify   thrpt   10  2826.806 ±  31.933  ops/ms

VS

after
Benchmark                        Mode  Cnt      Score     Error   Units
DatafakerExpressions.bothify    thrpt   10  10936.056 ±  39.143  ops/ms
DatafakerExpressions.letterify  thrpt   10  13036.411 ±  94.658  ops/ms
DatafakerExpressions.numerify   thrpt   10  13084.755 ± 423.533  ops/ms
DatafakerExpressions.regexify   thrpt   10   3541.279 ±   6.988  ops/ms

csv generation

before
Benchmark                  Mode  Cnt    Score   Error   Units
DatafakerCsvjson.csvJson  thrpt   10  301.604 ± 2.242  ops/ms

VS

after
Benchmark                  Mode  Cnt    Score    Error   Units
DatafakerCsvjson.csvJson  thrpt   10  377.927 ± 10.929  ops/ms

kotlin faker benchmark

before
Benchmark                                       Mode  Cnt    Score   Error  Units
Datafaker_KotlinFakerBenchmark.kotlinFakerTest  avgt   10  378.447 ± 1.685  ms/op

VS

after
Benchmark                                       Mode  Cnt    Score   Error  Units
Datafaker_KotlinFakerBenchmark.kotlinFakerTest  avgt   10  324.256 ± 2.438  ms/op

simple methods calls

before
Benchmark                              Mode  Cnt      Score      Error   Units
DatafakerSimpleMethods.firstname      thrpt   10   5838.567 ±  508.992  ops/ms
DatafakerSimpleMethods.fullname       thrpt   10   2429.474 ±  211.390  ops/ms
DatafakerSimpleMethods.numberBetween  thrpt   10  66001.570 ± 2144.033  ops/ms
DatafakerSimpleMethods.streetAddress  thrpt   10   1724.282 ±   62.587  ops/ms

VS

after
Benchmark                              Mode  Cnt       Score     Error   Units
DatafakerSimpleMethods.firstname      thrpt   10    8972.172 ±  99.469  ops/ms
DatafakerSimpleMethods.fullname       thrpt   10    3232.930 ± 203.367  ops/ms
DatafakerSimpleMethods.numberBetween  thrpt   10  102087.734 ± 655.514  ops/ms
DatafakerSimpleMethods.streetAddress  thrpt   10    2503.758 ±  19.876  ops/ms

@what-the-diff
Copy link

what-the-diff bot commented Apr 29, 2023

PR Summary

  • Bug fix in FakeValues.hashCode()
    Fixed an issue with the hashCode method in the FakeValues class
  • Enhanced hashCode methods
    Added hashCode implementations to both FakerContext and RegExpContext classes
  • Improved equals methods
    Updated equals methods for FakeValues, FakeValueService, and FakerContext classes for better precision and compatibility

@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2023

Codecov Report

Merging #792 (4f51016) into main (280729c) will decrease coverage by 0.38%.
The diff coverage is 65.95%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main     #792      +/-   ##
============================================
- Coverage     92.92%   92.54%   -0.38%     
- Complexity     2647     2654       +7     
============================================
  Files           285      286       +1     
  Lines          5216     5274      +58     
  Branches        537      553      +16     
============================================
+ Hits           4847     4881      +34     
- Misses          241      252      +11     
- Partials        128      141      +13     
Impacted Files Coverage Δ
.../main/java/net/datafaker/service/FakerContext.java 80.30% <20.00%> (-3.83%) ⬇️
...main/java/net/datafaker/service/RegExpContext.java 47.82% <47.82%> (ø)
.../java/net/datafaker/service/FakeValuesContext.java 80.85% <62.50%> (-4.45%) ⬇️
.../java/net/datafaker/service/FakeValuesService.java 83.98% <79.59%> (-0.38%) ⬇️
...rc/main/java/net/datafaker/service/FakeValues.java 84.61% <100.00%> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@snuyanzin snuyanzin requested a review from kingthorin April 29, 2023 15:18
Copy link
Collaborator

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants