Skip to content

SPR-16170 Spring-jdbc : Improve memory allocations when substituting named parameters. #1588

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

Merged
merged 1 commit into from
Nov 8, 2017

Conversation

benbenw
Copy link
Contributor

@benbenw benbenw commented Nov 7, 2017

Create the buffer with at least the original sql length to avoid
multiple re-allocations
Add a fast path if the original sql doesn't contain any parameters

JMH result

MyBenchmark.old                                                  thrpt   50   429702,315 ±   8526,336   ops/s
MyBenchmark.old:·gc.alloc.rate                                   thrpt   50     1089,447 ±     21,757  MB/sec
MyBenchmark.old:·gc.alloc.rate.norm                              thrpt   50     3992,001 ±      0,001    B/op
MyBenchmark.old:·gc.churn.PS_Eden_Space                          thrpt   50     1102,400 ±     29,500  MB/sec
MyBenchmark.old:·gc.churn.PS_Eden_Space.norm                     thrpt   50     4039,513 ±     71,742    B/op
MyBenchmark.old:·gc.churn.PS_Survivor_Space                      thrpt   50        0,180 ±      0,027  MB/sec
MyBenchmark.old:·gc.churn.PS_Survivor_Space.norm                 thrpt   50        0,659 ±      0,098    B/op
MyBenchmark.old:·gc.count                                        thrpt   50      783,000               counts
MyBenchmark.old:·gc.time                                         thrpt   50      333,000                   ms
MyBenchmark.newVersion                                           thrpt   50   478496,476 ±   5144,476   ops/s
MyBenchmark.newVersion:·gc.alloc.rate                            thrpt   50     1008,926 ±     10,849  MB/sec
MyBenchmark.newVersion:·gc.alloc.rate.norm                       thrpt   50     3320,001 ±      0,001    B/op
MyBenchmark.newVersion:·gc.churn.PS_Eden_Space                   thrpt   50     1023,339 ±     21,646  MB/sec
MyBenchmark.newVersion:·gc.churn.PS_Eden_Space.norm              thrpt   50     3368,042 ±     68,881    B/op
MyBenchmark.newVersion:·gc.churn.PS_Survivor_Space               thrpt   50        0,156 ±      0,033  MB/sec
MyBenchmark.newVersion:·gc.churn.PS_Survivor_Space.norm          thrpt   50        0,514 ±      0,109    B/op
MyBenchmark.newVersion:·gc.count                                 thrpt   50      647,000               counts
MyBenchmark.newVersion:·gc.time                                  thrpt   50      285,000                   ms

MyBenchmark.oldNoParams                                          thrpt   50  1642986,386 ±  47483,217   ops/s
MyBenchmark.oldNoParams:·gc.alloc.rate                           thrpt   50     2857,392 ±     82,456  MB/sec
MyBenchmark.oldNoParams:·gc.alloc.rate.norm                      thrpt   50     2738,400 ±      3,600    B/op
MyBenchmark.oldNoParams:·gc.churn.PS_Eden_Space                  thrpt   50     2893,399 ±     96,256  MB/sec
MyBenchmark.oldNoParams:·gc.churn.PS_Eden_Space.norm             thrpt   50     2773,357 ±     51,150    B/op
MyBenchmark.oldNoParams:·gc.churn.PS_Survivor_Space              thrpt   50        0,186 ±      0,031  MB/sec
MyBenchmark.oldNoParams:·gc.churn.PS_Survivor_Space.norm         thrpt   50        0,178 ±      0,029    B/op
MyBenchmark.oldNoParams:·gc.count                                thrpt   50      882,000               counts
MyBenchmark.oldNoParams:·gc.time                                 thrpt   50      382,000                   ms
MyBenchmark.newVersionNoParams                                   thrpt   50  2355892,983 ± 107644,968   ops/s
MyBenchmark.newVersionNoParams:·gc.alloc.rate                    thrpt   50     2178,567 ±     99,614  MB/sec
MyBenchmark.newVersionNoParams:·gc.alloc.rate.norm               thrpt   50     1456,000 ±      0,001    B/op
MyBenchmark.newVersionNoParams:·gc.churn.PS_Eden_Space           thrpt   50     2186,719 ±    112,842  MB/sec
MyBenchmark.newVersionNoParams:·gc.churn.PS_Eden_Space.norm      thrpt   50     1461,406 ±     35,027    B/op
MyBenchmark.newVersionNoParams:·gc.churn.PS_Survivor_Space       thrpt   50        0,179 ±      0,030  MB/sec
MyBenchmark.newVersionNoParams:·gc.churn.PS_Survivor_Space.norm  thrpt   50        0,121 ±      0,022    B/op
MyBenchmark.newVersionNoParams:·gc.count                         thrpt   50      907,000               counts
MyBenchmark.newVersionNoParams:·gc.time                          thrpt   50      401,000                   ms

List<String> paramNames = parsedSql.getParameterNames();
if(paramNames.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor thing: According to our code style, the "if" needs a space before the bracket, i.e. "if (paramNames.isEmpty())". If you could fix this quickly, I'd be happy to merge your pull request unchanged. I'll care for the backport to 4.3.x towards the end of this week then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.
Thx for the backport

@jhoeller
Copy link
Contributor

jhoeller commented Nov 8, 2017

One more minor thing: Could you please squash those two commits into one? I can also squash and merge them on my end but we usually prefer a single pre-squashed pull request that we can merge as-is. Not least of it all, that allows us to preserve your contribution in its most original form...

Create the buffer with at least the original sql length to avoid
multiple re-allocations
Add a fast path if the original sql doesn't contain any parameters
@benbenw
Copy link
Contributor Author

benbenw commented Nov 8, 2017

Commits squashed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants