Fix performance bug with large number of unnamed parameters #2050
+4
−2
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Preamble:
Consider the following sql script:
select some from table where id in (:args)
Where args is a 10k items list
It does not look good, but people do it anyways, or maybe just when the system scales up, and it slowly grows from 100 items to 10k items
The problem
In such conditions QueryMapper.getUniqueName needs to figure out the names for each of the elements in the
in (:a1,:a2,:a3, :a10000)
list, it does so by generating a name by counter and tries to lookup the name in the parameters hashmapBut if the argument list is so large it traverses the map over and over again
Ultimately it results in O(N^2) complexity (where operation behind is a hashmap lookup)
The symptom is a very high CPU consumption, and it is really slow
The fix
The suggested fix is just to skip most of the counter-hashmap traversal, just make the counter go forward
We still need the loop to guarantee generated names are unique
More on the problem
While 10k case is just very obvious to see and detect but also
My guess that likely a lot of code out there, having 50+ of arguments invisibly suffer and the fix should probably help them too
I'm sorry about no tests, the problem here that the nature of the bug is whether it works slow, I'm not sure if there are any load tests or something for this repo