Avoid quadratic removeIf in JsonArray list views - #3115
Open
ishu7w wants to merge 3 commits into
Open
Conversation
Delegate removeIf to the backing ArrayList bulk-removal implementation.
Cover duplicate matches, JSON nulls, preserved order, empty arrays, and null predicates.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Purpose
Avoid quadratic time when removing many elements through
JsonArray.asList().removeIf(...).Description
NonNullElementWrapperListinheritsCollection.removeIf, which removes matching elements through the iterator. Each removal shifts the tail of the backingArrayList. Filtering out a large fraction of an array therefore repeatedly copies the remaining elements.Delegate
removeIfto the backingArrayListso it uses bulk removal, consistent with the existingremoveAllandretainAlloverrides. No dependency or public API is added.The tests cover duplicate matches, JSON nulls, remaining element order, all/no matches, empty arrays, null predicates, and updates to the original
JsonArray.This also follows
ArrayListbehavior if the predicate throws: the operation may leave the list unchanged instead of partially filtered. TheCollectioncontract does not guarantee a particular partial result on exception.Validation
mvn -B -ntp clean verify javadoc:jarpassed across all eight modules.JsonArrayAsListTestandJsonArrayAsListSuiteTest: 422 tests passed.A local microbenchmark compiled the wrapper source before and after the change, populated an
ArrayList<Integer>with consecutive integers, and timednew NonNullElementWrapperList<>(backing).removeIf(value -> (value & 1) == 0). Array construction was outside the timed interval. After ten 10,000-element warmups, medians of seven samples were:These are illustrative single-machine measurements, not a JMH benchmark or an end-to-end application speed guarantee. The added functional tests verify behavior; they do not use timing assertions.
Checklist
mvn clean verify javadoc:jarpasses.@since: not applicable; no new public API.AI assistance
Codex identified the performance issue, prepared the implementation and tests, ran local validation, and drafted this description.