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

Optimize AntPathMatcher when checking for potential matches [SPR-15477] #20037

Closed
spring-issuemaster opened this issue Apr 24, 2017 · 5 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Apr 24, 2017

Christoph Dreis opened SPR-15477 and commented

Hey,

just noticed a possible improvement when checking for potential matches in AntPathMatcher. The problem here is that toCharArray() is called in isPotentialMatch() and skipSegment() which clones the array under the hood and causes allocations that we could avoid.

The attached patch shows a possible solution with simply using charAt() that leads to the following benchmark results. Would be happy if this is accepted.

New

Benchmark                                      Mode  Cnt         Score         Error   Units
Benchmark.testNew                      thrpt   20  43196627,181 ± 1235654,920   ops/s
Benchmark.testNew:·gc.alloc.rate       thrpt   20         0,001 ±       0,002  MB/sec
Benchmark.testNew:·gc.alloc.rate.norm  thrpt   20        ? 10??                  B/op
Benchmark.testNew:·gc.count            thrpt   20           ? 0                counts

Old

Benchmark                                                   Mode  Cnt         Score        Error   Units
Benchmark.testOld                                   thrpt   20  29555122,576 ± 507145,389   ops/s
Benchmark.testOld:·gc.alloc.rate                    thrpt   20      1052,083 ±     18,027  MB/sec
Benchmark.testOld:·gc.alloc.rate.norm               thrpt   20        56,000 ±      0,001    B/op
Benchmark.testOld:·gc.churn.PS_Eden_Space           thrpt   20      1050,787 ±     28,954  MB/sec
Benchmark.testOld:·gc.churn.PS_Eden_Space.norm      thrpt   20        55,927 ±      1,048    B/op
Benchmark.testOld:·gc.churn.PS_Survivor_Space       thrpt   20         0,190 ±      0,046  MB/sec
Benchmark.testOld:·gc.churn.PS_Survivor_Space.norm  thrpt   20         0,010 ±      0,002    B/op
Benchmark.testOld:·gc.count                         thrpt   20       354,000               counts
Benchmark.testOld:·gc.time                          thrpt   20       182,000                   ms

Cheers,
Christoph


Affects: 4.3.8

Referenced from: pull request #1399, and commits baa7b1c, 13b3952, cd95f34, 84d2e5a

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 27, 2017

Juergen Hoeller commented

Optimizing local computations as well, I guess this would be even better:

private int skipSegment(String path, int pos, String prefix) {
     int skipped = 0;
     for (int i = 0; i < prefix.length(); i++) {
          char c = prefix.charAt(i);
          if (isWildcardChar(c)) {
               return skipped;
          }
          int currPos = pos + skipped;
          if (currPos >= path.length()) {
               return 0;
          }
          if (c == path.charAt(currPos)) {
               skipped++;
          }
     }
     return skipped;
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 27, 2017

Juergen Hoeller commented

Feel free to update your PR accordingly, we can merge it straight away then. Otherwise I can also proceed with my local changes if you prefer.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 27, 2017

Christoph Dreis commented

Thanks for looking into this. Jep, you're right - we could save a bit more. For completeness reasons the newest benchmark:

Benchmark.testNew                      thrpt   20  43428506,223 ± 542779,409   ops/s
Benchmark.testNew:·gc.alloc.rate       thrpt   20         0,001 ±      0,002  MB/sec
Benchmark.testNew:·gc.alloc.rate.norm  thrpt   20        ? 10??                 B/op
Benchmark.testNew:·gc.count            thrpt   20           ? 0               counts

I've updated the PR accordingly.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 27, 2017

Juergen Hoeller commented

Alright, here we go then :-) I'll backport it to 4.3.x tomorrow.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 27, 2017

Christoph Dreis commented

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.