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

GenericSchema::schemaMap_ is not a map and leads to O(n) complexity #2286

Open
svart-riddare opened this issue Jun 6, 2024 · 4 comments
Open

Comments

@svart-riddare
Copy link
Contributor

Upon investigation of performance issues, we found out that GenericSchema::schemaMap_ is a linear type container and thus methods GenericSchema::GetSchema() and GenericSchema::GetPointer() have O(n) complexity, yielding a huge performance impact on our application.

@aikawayataro
Copy link
Contributor

This is indeed a map. Map (associative array/dictionary, etc.) has no implementation constraints. A map with a hash table is probably what you want to see.
But I suspect that nothing is going to change. Are you really able to confirm that GetPointer is the culprit for the performance issues?

@svart-riddare
Copy link
Contributor Author

You are right, my wording was not precise enough, by map I meant "efficient" map !

Yes, I used the perf tool to confirm that more than 60% of the time was spent in those functions; I bluntly inserted a std::map for GenericSchema::schemaMap_ and I got a x3 speedup and those functions disappeared from perf report.

@aikawayataro
Copy link
Contributor

If it's 60% of a large complex application, it's certainly a problem.
I believe you can implement your own SchemaDocument using the reference implementation but with std::map or std::hash_map. Or you can just patch the code :) .

@svart-riddare
Copy link
Contributor Author

Yes, that's what I did; there's no release/tag of rapidjson since 2016 anyway. I opened a ticket to let other know.

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

No branches or pull requests

2 participants