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

DAO/SqlObject interface performance #2617

Open
johnarrr opened this issue Jan 28, 2024 · 8 comments
Open

DAO/SqlObject interface performance #2617

johnarrr opened this issue Jan 28, 2024 · 8 comments

Comments

@johnarrr
Copy link

Background: I've been working to convert an application from jdbi2 to jdbi3. One challenge has been performance. The current jdbi2 app uses declarative syntax with DAO classes. The same approach in jdbi3 is 2-3x slower. The jdbi3 fluent interface timings are roughly the same as jdbi2. Performance testing with 25k iterations seems to be consistent across select, insert, and update operations.

This is similar to what others have highlighted, especially #2601 and #2406. If it helps, I created a project with the 3 different approaches:
https://github.com/johnarrr/jdbi_perf_test
The results above are with Java 11.

Digging around with a profiler and debugger, one thing I noticed is every call to the DAO method triggers rediscovery of the mappers. It looks like mappers are cached, but only within the scope of a DAO method call. Specifically, the mapper cache is a couple of layers inside ConfigRegistry. ExtensionHandlerInvoker creates a new copy of the config, which resets the cache. See ExtensionMetadata::createMethodConfiguration

If this is accurate, is it supposed to work like that? Is there anything in my DAO or jdbi connection setup that causes this?

@johnarrr
Copy link
Author

johnarrr commented Jan 29, 2024

FWIW, a guava ConfigRegistry cache in ExtensionMetadata reduced the runtime by about 25% for a lot of SELECT queries. The same test with fluent calls is about 50% below the runtime of unmodified declarative calls.

@stevenschlansker
Copy link
Member

Thank you for this report. I want to revisit the performance problems that pile up around Config. Unfortunately, I think it is a larger project.

It was a mistake to have mutable config objects that the user can edit at any time. This means we have to defensively copy all config, in case someone in another thread reconfigures concurrently.

My current thought is that we introduce a new path to create Handle and Dao instances that looks more like a builder.build(), where you promise (and maybe we enforce?) that config will not be edited later, and is safe to share without copying.

I cannot promise any particular timeframe on my work on this, but it is near the top of my personal pet project list.

@johnarrr
Copy link
Author

Thank you for the note. Sorry to pile onto the topic, and I understand the need for a longer time horizon for these changes.

The use of DAOs and annotations implies compile-time decisions on the app/consumer's part. I would expect jdbi changes to require a recompile + redeploy and not support runtime changes. Making the config immutable seems like a natural extension of that principle.

Similarly, is it feasible to perform a broader one-time setup as DAO classes are instantiated? We use guice to init all persistence layer objects on app init, so everything should be resident by the time user activity is handled. It looks like warm() calls are currently lazy. Could the init process be forced, so DAO method calls are more direct during normal runtime operations? e.g. bypass the factories and assume everything already exists

Please share if there is anything I can do that would help.

@hgschmie
Copy link
Contributor

I put a "copy-on-write caching" thing together a while back (I think that inspired #2406). I can dig that out and put a side branch for you to pull and test (would need to compile yourself however). LMK if that is something that you want to do.

The problem with the registry is that it mixes two concepts: The registration of things like mappers, which we can address in builder style as explained by @stevenschlansker but then we also have listener configuration (which you want to be able to register and unregister at runtime) and other parameters that want to be runtime-managed.

Having an immutable (and if possible aggressively pre-built) cache for things that are stateless and immutable would be a major performance gain. However, we have been very very careful to maintain full backwards compatibility (the Jdbi community is small and introducing major breaking changes would make it smaller) which limits our options here.

There are thoughts around this for Jdbi 4 but that is a major effort where we may need to look for corporate sponsorship.

@stevenschlansker
Copy link
Member

Let's not start the "v4" discussion until we have to. Until we find otherwise, I still hope these issues can be fixed or at least improved in v3. That might involve new APIs but of course we would try not to break anything.

@johnarrr
Copy link
Author

Thanks for the additional background and info. I am happy to test potential changes if that would help.

@stevenschlansker
Copy link
Member

@johnarrr , one thing you could contribute in the meantime if you are inspired is additional benchmark/ tests to the project to exhibit the problems you're experiencing.

@stevenschlansker
Copy link
Member

Actually, your test cases look potentially pretty similar to the ones that are already there!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants