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

[Profiling] Map stack frames more efficiently #94327

Merged

Conversation

danielmitterdorfer
Copy link
Member

The mapping code for stack frames uses the utility method ObjectPath#eval to read nested properties. Callers need to pass a dot-separated path which is then split internally via a regex. This is quite slow: in a typical deployment we saw overheads of 50ms just for mapping stack frames (total response time is ~ 1 second).

With this commit we pass the property path as an array to avoid this overhead. In a microbenchmark the new implementation was 23 times faster than the current one.

The mapping code for stack frames uses the utility method
`ObjectPath#eval` to read nested properties. Callers need to pass a
dot-separated path which is then split internally via a regex. This is
quite slow: in a typical deployment we saw overheads of 50ms just for
mapping stack frames (total response time is ~ 1 second).

With this commit we pass the property path as an array to avoid this
overhead. In a microbenchmark the new implementation was 23 times faster
than the current one.
@danielmitterdorfer danielmitterdorfer added >enhancement :Search/Search Search-related issues that do not fall into other categories v8.8.0 labels Mar 6, 2023
@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Mar 6, 2023
@elasticsearchmachine
Copy link
Collaborator

Hi @danielmitterdorfer, I've created a changelog YAML for you.

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@danielmitterdorfer
Copy link
Member Author

Below are the results of a microbenchmark with JMH's gc profiler enabled. We can see that the new implementation pathFromArray is ~ 23 times faster than pathFromString. Additionally, it does not cause any allocations:

Benchmark                                                             Mode  Cnt     Score     Error   Units
ObjectPathBenchmark.pathFromArray                                    thrpt   30   385.025 ±   0.888  ops/us
ObjectPathBenchmark.pathFromArray:·gc.alloc.rate                     thrpt   30    ≈ 10⁻⁴            MB/sec
ObjectPathBenchmark.pathFromArray:·gc.alloc.rate.norm                thrpt   30    ≈ 10⁻⁷              B/op
ObjectPathBenchmark.pathFromArray:·gc.count                          thrpt   30       ≈ 0            counts
ObjectPathBenchmark.pathFromString                                   thrpt   30    16.230 ±   1.733  ops/us
ObjectPathBenchmark.pathFromString:·gc.alloc.rate                    thrpt   30  3537.559 ± 377.722  MB/sec
ObjectPathBenchmark.pathFromString:·gc.alloc.rate.norm               thrpt   30   240.010 ±   0.001    B/op
ObjectPathBenchmark.pathFromString:·gc.churn.G1_Eden_Space           thrpt   30  3540.005 ± 379.118  MB/sec
ObjectPathBenchmark.pathFromString:·gc.churn.G1_Eden_Space.norm      thrpt   30   240.156 ±   0.687    B/op
ObjectPathBenchmark.pathFromString:·gc.churn.G1_Survivor_Space       thrpt   30     0.014 ±   0.003  MB/sec
ObjectPathBenchmark.pathFromString:·gc.churn.G1_Survivor_Space.norm  thrpt   30     0.001 ±   0.001    B/op
ObjectPathBenchmark.pathFromString:·gc.count                         thrpt   30  2310.000            counts
ObjectPathBenchmark.pathFromString:·gc.time                          thrpt   30  1785.000                ms

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

Makes sense to me! Creating a string only to split it again is a ton of useless work.

@danielmitterdorfer
Copy link
Member Author

Thanks for the quick review!

@danielmitterdorfer danielmitterdorfer merged commit 299eff5 into elastic:main Mar 6, 2023
@danielmitterdorfer danielmitterdorfer deleted the profiling-eval-path branch March 6, 2023 14:57
saarikabhasi pushed a commit to saarikabhasi/elasticsearch that referenced this pull request Apr 10, 2023
The mapping code for stack frames uses the utility method
`ObjectPath#eval` to read nested properties. Callers need to pass a
dot-separated path which is then split internally via a regex. This is
quite slow: in a typical deployment we saw overheads of 50ms just for
mapping stack frames (total response time is ~ 1 second).

With this commit we pass the property path as an array to avoid this
overhead. In a microbenchmark the new implementation was 23 times faster
than the current one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants