Hi,
here is another PR that tries to improve the binding of extremely large files as reported in #16401.
There are basically two main ideas behind the proposed changes:
- Change
isAncestorOf()
so it is more likely inlined. - When profiling the provided app in #16401 it became apparent that the element equality checks compare mostly a combination of
UNIFORM
,NUMERICALLY_INDEXED
andDASHED
elements. All of thoseElementType
s don't really need any lowercasing or skipping of non alphanumeric chars. Specifically, the first two could benefit from a fast version.DASHED
- described as "almost" uniform - can benefit of a trimmed down version of the current checks as well.
The proposed changes provide the following results with @wilkinsona`s benchmark harness:
Baseline | New | |
---|---|---|
15.109 | 12.921 | |
13.843 | 12.042 | |
13.451 | 11.900 | |
12.736 | 11.975 | |
12.712 | 11.945 | |
14.475 | 12.289 | |
13.132 | 11.821 | |
13.066 | 12.177 | |
12.261 | 12.343 | |
12.940 | 12.048 | |
Mean | 13.372 | 12.146 |
Range | 2.848 | 1.100 |
When running an adjusted version of the JMH benchmarks provided in #15760 I get the following results: Before
Benchmark (prof) Mode Cnt Score Error Units
PropertiesBenchmarkIT.auto medium thrpt 10 194,016 ± 107,644 ops/s
PropertiesBenchmarkIT.auto:size medium thrpt 10 1340,000 #
PropertiesBenchmarkIT.auto small thrpt 10 13102,403 ± 6903,192 ops/s
PropertiesBenchmarkIT.auto:size small thrpt 10 10,000 #
PropertiesBenchmarkIT.auto large thrpt 10 2,886 ± 0,980 ops/s
PropertiesBenchmarkIT.auto:size large thrpt 10 10930,000 #
PropertiesBenchmarkIT.auto verylarge thrpt 10 0,119 ± 0,016 ops/s
PropertiesBenchmarkIT.auto:size verylarge thrpt 10 50000,000 #
After
Benchmark (prof) Mode Cnt Score Error Units
PropertiesBenchmarkIT.auto medium thrpt 10 203,827 ± 104,316 ops/s
PropertiesBenchmarkIT.auto:size medium thrpt 10 1340,000 #
PropertiesBenchmarkIT.auto small thrpt 10 15183,887 ± 5609,828 ops/s
PropertiesBenchmarkIT.auto:size small thrpt 10 10,000 #
PropertiesBenchmarkIT.auto large thrpt 10 3,015 ± 0,898 ops/s
PropertiesBenchmarkIT.auto:size large thrpt 10 10930,000 #
PropertiesBenchmarkIT.auto verylarge thrpt 10 0,147 ± 0,009 ops/s
PropertiesBenchmarkIT.auto:size verylarge thrpt 10 50000,000 #
Overall, I think these are promising results. Let me know what you think.
Cheers, Christoph
Comment From: dreis2211
Test failure seems unrelated
Comment From: dreis2211
@wilkinsona Any chance you find some time to look into this one? Would be highly appreciated. (I'm obviously excited about your thoughts :D )
Comment From: wilkinsona
Sure, I'll take a look this week. I did take a bit of look before my Easter break and lacked the brain power for a proper review!
Comment From: wilkinsona
@dreis2211 Thanks very much for this. I've had the time (and brain power) to hopefully give things a proper review and the changes seem good to me. I spent some time wondering if we needed some more tests, but things already seem to be covered quite nicely by the existing ConfigurationPropertyNameTests.equalsAndHashCode()
.
Comment From: dreis2211
Thanks for the review. ConfigurationPropertyNameTests.equalsAndHashCode()
did indeed fail while I worked on this, so I figured the same.
Comment From: wilkinsona
Thanks once again, @dreis2211. The proposed changes have been merged into master.
Comment From: Shawyeok
@wilkinsona @dreis2211
Nice work!
I'm face this problem too, but currently my project still use spring-boot 2.1.x
, is 2.1.x
under maintenance? Can this improvement pick to 2.1.x
branch?
Comment From: wilkinsona
I'm afraid not, no. As announced at the end of 2019, Spring Boot 2.1.x reached the end of its supported life in November 2020. You should upgrade to Spring Boot 2.4.x or 2.5.x at your earliest convenience.
Comment From: Shawyeok
I'm afraid not, no. As announced at the end of 2019, Spring Boot 2.1.x reached the end of its supported life in November 2020. You should upgrade to Spring Boot 2.4.x or 2.5.x at your earliest convenience.
I see, thanks.