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

Remove synchronization from ResourceBundleMessageSource [SPR-16235] #20782

Closed
spring-projects-issues opened this issue Nov 26, 2017 · 2 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Nov 26, 2017

Masahiro Ide opened SPR-16235 and commented

The ResourceBundleMessageSource class caches the properties to HashMaps by default and
this class locks the hashmaps when getting/putting a entry from/to hashmaps. This synchronization causes slow down the property look-up in multi threaded application.

Here is the stack trace, notice the first line is holding lock on a hashmap, which is the hashmap that holds properties:

org.springframework.context.support.ResourceBundleMessageSource.getResourceBundle(String, Locale) Line: 176
org.springframework.context.support.ResourceBundleMessageSource.resolveCodeWithoutArguments(String, Locale) Line: 129
org.springframework.context.support.AbstractMessageSource.getMessageInternal(String, Object[], Locale) Line: 209
org.springframework.context.support.AbstractMessageSource.getMessage(String, Object[], Locale) Line: 151
org.springframework.context.support.AbstractApplicationContext.getMessage(String, Object[], Locale) Line: 1256
...

Because in major case, these hashmaps are ready-heavy and entries of these hashmaps will not be purged. To improve this, want to use a concurrent hashmap.

Here is the patch, jmh benchamrk and benchmark results.

1. Thread=1
Benchmark                                         Mode  Cnt        Score       Error  Units
ResourceBundleMessageSourceBenchmark.concurrent  thrpt   20  2460083.135 ± 21356.649  ops/s
ResourceBundleMessageSourceBenchmark.original    thrpt   20  2628372.407 ± 31603.771  ops/s
1. Thread=20
Benchmark                                         Mode  Cnt         Score        Error  Units
ResourceBundleMessageSourceBenchmark.concurrent  thrpt   20  11137855.564 ± 344640.055  ops/s
ResourceBundleMessageSourceBenchmark.original    thrpt   20   1924866.006 ± 332692.104  ops/s

As you can see, if my change is correct, using ConcurrentHashMap is 5~6X faster than large synchronization block.


Affects: 4.3.12, 5.0.1

Reference URL: https://github.com/imasahiro/spring-issue

Issue Links:

Referenced from: pull request #1605, and commits d9af4d6

@spring-projects-issues
Copy link
Collaborator Author

Masahiro Ide commented

Filed a PR - #1605

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 9, 2018

Juergen Hoeller commented

Good point, this finally brings ResourceBundleMessageSource in line with the current implementation style of ReloadableResourceBundleMessageSource (as of #15133)...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants