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

[BUG/performance] Serialization/Deserialization check for safe classes adds performance overhead #4760

Closed
krishna-ggk opened this issue Sep 30, 2024 · 1 comment · Fixed by #4973
Labels
bug Something isn't working triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. v2.19.0 Issues targeting release v2.19.0

Comments

@krishna-ggk
Copy link
Contributor

What is the bug?
While debugging latency contributors in an OpenSearch 2.13 cluster using JDK serialization, we noticed SafeSerializationUtils.isSafeClass adds ~20% overhead to latency although it isn't as visible in flamegraphs (0.02%) (This was validated by short circuiting isSafeClass to return).

image

image

What is the expected behavior?
It seems like a very low hanging fruit to gain significant performance benefit.

@krishna-ggk krishna-ggk added bug Something isn't working untriaged Require the attention of the repository maintainers and may need to be prioritized labels Sep 30, 2024
@krishna-ggk krishna-ggk changed the title [BUG/performance] Safe classes to serialize/deserialize adds performance overhead in high volume scenarios [BUG/performance] Serialization/Deserialization check for safe classes adds performance overhead Sep 30, 2024
@cwperks cwperks added triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels Oct 7, 2024
@cwperks
Copy link
Member

cwperks commented Oct 7, 2024

[Triage] Thanks for the issue @krishna-ggk and associated flame graph. 20% overhead sounds quite high for a method that is just to lookup a classname in a list of allowed class names. This sounds like it would be a good performance benefit if it can be optimized securely. Marking this issue as triaged.

@cwperks cwperks added the v2.19.0 Issues targeting release v2.19.0 label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. v2.19.0 Issues targeting release v2.19.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants