Skip to content

feat: Optimize DatabaseDriver fromJdbcUrl method with caching #43638

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

Closed
wants to merge 1 commit into from

Conversation

minwoo1999
Copy link
Contributor

This PR introduces a cache (URL_PREFIX_CACHE) to store URL prefixes for all DatabaseDriver enums. By precomputing prefixes during initialization, the fromJdbcUrl method avoids redundant calls to getUrlPrefixes, significantly improving performance for repeated lookups.

Caching was chosen to reduce computational overhead while keeping the code maintainable. Future updates to getUrlPrefixes automatically reflect in the cache, requiring no additional changes.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 30, 2024
@snicoll
Copy link
Member

snicoll commented Dec 30, 2024

By precomputing prefixes during initialization, the fromJdbcUrl method avoids redundant calls to getUrlPrefixes, significantly improving performance for repeated lookups.

What would be the reason for calling this method repeatedly?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Dec 30, 2024
@minwoo1999
Copy link
Contributor Author

By precomputing prefixes during initialization, the fromJdbcUrl method avoids redundant calls to getUrlPrefixes, significantly improving performance for repeated lookups.

What would be the reason for calling this method repeatedly?

In my opinion, the fromJdbcUrl method is frequently invoked in applications that dynamically manage database connections. This is especially true in multi-tenant systems or environments with multiple data sources, where determining the correct database driver for each connection is essential.

Therefore, by precomputing and caching URL prefixes in URL_PREFIX_CACHE, the method avoids recalculating them repeatedly, leading to significantly faster and more efficient driver resolution. This optimization is particularly advantageous in high-throughput, scalable environments, ensuring smoother performance when handling large numbers of database interactions.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 30, 2024
@philwebb
Copy link
Member

philwebb commented Dec 30, 2024

I don't really see how this will improve the performance that much. Calling getUrlPrefixes() shouldn't be expensive, and if we did want to optimize it we could make the result a static final constant which would save the one call to List.of(...).

@minwoo1999 Have you actually encountered performance issues? Have you benchmarked your changes?

@minwoo1999
Copy link
Contributor Author

I don't really see how this will improve the performance that much. Calling getUrlPrefixes() shouldn't be expensive, and if we did want to optimize it we could make the result a static final constant which would save the one call to List.of(...).

@minwoo1999 Have you actually encountered performance issues? Have you benchmarked your changes?

I haven’t encountered specific performance issues or conducted benchmarks related to this change. However, the fromJdbcUrl method is commonly used in scenarios where database connections need to be dynamically determined at runtime. For instance, in multi-tenant systems, each tenant might have its own database, and the application must resolve the correct driver for each request. Similarly, applications managing multiple data sources rely on this method to handle various JDBC URLs effectively.

In addition, runtime configuration changes, such as switching between development, testing, or production environments, often require dynamic connection updates. In sharded or distributed database setups, this method is also essential for routing queries to the appropriate database instance.

This optimization was made with these scenarios in mind, aiming to make the process more efficient and scalable. While no immediate performance bottleneck was observed, the goal was to proactively improve the method’s efficiency for systems with high database interaction or dynamic configurations.

@philwebb
Copy link
Member

Thanks for the PR, but without actually encountering performance issues or seeing benchmark results this feels like premature optimization to me.

@philwebb philwebb closed this Dec 30, 2024
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants