Skip to content
New issue

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

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

Already on GitHub? # to your account

0.9.0 introduces massive performance issues #28

Open
eygraber opened this issue Nov 9, 2022 · 6 comments
Open

0.9.0 introduces massive performance issues #28

eygraber opened this issue Nov 9, 2022 · 6 comments

Comments

@eygraber
Copy link

eygraber commented Nov 9, 2022

Updating from 0.8.0 to 0.9.0 is introducing massive performance issues in benchmarks that I'm running. Ranges from 100-1000X slower with 0.9.0

@ychescale9
Copy link
Member

Can you share more about the benchmark? Is it on jvm, native or both?
Which operations are slower?

@eygraber
Copy link
Author

eygraber commented Nov 9, 2022

It's on jvm. I tried updating in middle of a big refactor and I rolled back now. I'll have more info once I finish the refactor and I can try again.

@ychescale9
Copy link
Member

Thanks! Any chance you can share the benchmark in a repo?

The original implementation in dropbox/store was jvm only and performance was reasonable when I benchmarked against some of the other caching library.

When I moved it to this repo to make it KMP, I swapped the internals to use stately which supports KMP and is thread-safe on jvm.

A few releases ago @dcvz contributed a change that replaces stately's collections with kotlin's mutable collections which aren't thread safe and a few jvm users reported concurrent modification exception. 0.9.0 reverted back to using stately to bring thread safety back so the performance issues you're seeing were likely present in the earlier releases too.

One thing we could try is completely separate jvm and non jvm implementation and revert back to the original jvm only implementation for jvm targets and use stately for native targets. The other option to explore is use AndroidX collection KMP for all the supported targets (jvm + iOS I believe) and use stately for everything else.

But it'd be good to have a benchmark project to verify any performance impacts with these changes so if you have something we can use as a baseline or even contribute to this repo that'd be amazing 🤩

@eygraber
Copy link
Author

If you run runBenchmarks.sh from https://github.com/eygraber/JsonPathKt/tree/kmp you can see the cache performance is pretty good (cache4k timing is printed on the left side of the table).

If you then bump cache4k to 0.9.0 and run runBenchmarks.sh again, you can see the timing with the cache is dramatically different.

@eygraber
Copy link
Author

I attached a YourKit profile that seems to show that Stately is the issue here (at least for jvm; I haven't tested the other targets)

BenchmarkTest-2022-11-10.zip

@ychescale9
Copy link
Member

Thanks! Will look into it.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants