Skip to content

Improve Binder performance slightly #20755

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

Conversation

dreis2211
Copy link
Contributor

Hi,

I'm currently trying to improve the startup of a medium sized app and found a couple of minor optimization opportunities in the Binder logic:

  1. Skip looking for descendants if context.depth is 0. (Was there before, but only as a subsequent condition)
  2. Defer overhead of getting ConfigurationPropertys in IndexedElementsBinder:: getKnownIndexedChildren until it's needed. The idea is that this is only really necessary if there are unbound properties - which should be the exception rather than the norm.

Applying this patch shows me the following results for the initializr benchmarks we used in the past already:

2.3.0.M3
Benchmark                        (prof)   Mode  Cnt      Score      Error  Units
PropertiesBenchmarkIT.auto       medium  thrpt   10    238,644 ±  138,881  ops/s
PropertiesBenchmarkIT.auto:size  medium  thrpt   10   1340,000                 #
PropertiesBenchmarkIT.auto        small  thrpt   10  16863,321 ± 8881,464  ops/s
PropertiesBenchmarkIT.auto:size   small  thrpt   10     10,000                 #
PropertiesBenchmarkIT.auto        large  thrpt   10      4,492 ±    0,647  ops/s
PropertiesBenchmarkIT.auto:size   large  thrpt   10  10930,000                 #

Patch
Benchmark                        (prof)   Mode  Cnt      Score      Error  Units
PropertiesBenchmarkIT.auto       medium  thrpt   10    275,289 ±  144,481  ops/s
PropertiesBenchmarkIT.auto:size  medium  thrpt   10   1340,000                 #
PropertiesBenchmarkIT.auto        small  thrpt   10  20426,557 ± 4507,749  ops/s
PropertiesBenchmarkIT.auto:size   small  thrpt   10     10,000                 #
PropertiesBenchmarkIT.auto        large  thrpt   10      5,121 ±    0,428  ops/s
PropertiesBenchmarkIT.auto:size   large  thrpt   10  10930,000                 #

I also ran 2.3.0.M3 vs. this patch against the config example which we used in the past to optimize the binding of large properties:

2.3.0.M3 Patch
4.986 4.790
4.715 4.682
4.833 4.651
4.719 4.762
5.161 4.743
4.830 4.744
5.081 4.791
5.850 4.715
4.767 4.803
4.877 4.804
Mean 4.982 4.749
Range 1.135 0.153

I should note that I see relatively volatile timings on my local machine, but always in favor of the new patched version. Nonetheless, I'd give it a try before merging this if I were you - even if the improvements seem relatively straight-forward ;-)

Let me know what you think.
Cheers,
Christoph

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 31, 2020
@dreis2211
Copy link
Contributor Author

Test failure seems unrelated.

@wilkinsona
Copy link
Member

Test failure seems unrelated

It looks like another occurrence of #20790. Hopefully we won't see that one any more 🤞.

@snicoll snicoll added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 10, 2020
@snicoll snicoll self-assigned this Apr 10, 2020
@snicoll snicoll added this to the 2.3.0.RC1 milestone Apr 10, 2020
snicoll pushed a commit that referenced this pull request Apr 10, 2020
@snicoll snicoll closed this in 2ab2844 Apr 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants