configcat / java-sdk

ConfigCat SDK for Java. ConfigCat is a hosted feature flag service: https://configcat.com. Manage feature toggles across frontend, backend, mobile, desktop apps. Alternative to LaunchDarkly. Management app + feature flag SDKs.
https://configcat.com/docs/sdk-reference/java
MIT License
20 stars 6 forks source link

Replace synchronization with a ReentrantReadWriteLock in ConfigCatHooks #49

Closed croemmich closed 1 month ago

croemmich commented 2 months ago

The ConfigCat client is thread safe and largely scales evaluations across threads, however, hook executions are all synchronized. This can seriously impact performance if the onFlagEvaluated hook(s) performs any significant logic (even small 100s of microseconds) or could cause evaluations to hang if a long running onConfigChanged hook is executed. A ReentrantReadWriteLock resolves this by allowing multiple concurrent readers to iterate the hook lists.

Describe the purpose of your pull request

Other Notes

While this change does improve performance pretty dramatically when using hooks, some effort should be considered to further optimize the locking in ConfigService.fetchIfOlder and ConfigService.processResponse, which can still result in significant latency spikes when loading new config, especially when remote caches are used, or if the onConfigChanged hook(s) contain any significant logic.

It could also make sense in a future library version (since it would be a breaking change) to makes hooks immutable outside of the builder to allow for removing the locks entirely. An implementor could create a mutable wrapper if they needed to modify hooks after the client is created.

Requirement checklist (only if applicable)

z4kn4fein commented 1 month ago

Thank you for the contribution!