-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Optimize ObjectNode findValue(s) and findParent(s) fast paths
#4008
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ObjectNode findValue(s) and findParent(s) fast paths
|
Thank you @schlosna ! Merged for inclusion in 2.16.0 |
|
This is super clean improvement 👍🏻 May I ask just one question tho, in what cases could traveral (the original way) still necessary? |
|
Lookups are recursive, i.e. find at any level, not just current one. For that it's still needed. |
|
Excellent, thanks @cowtowncoder ! |
Ahhh, I totally missed that. Thank you for the explanation! 🙏🏼 |
| foundSoFar = new ArrayList<>(); | ||
| } | ||
| foundSoFar.add(jsonNode); | ||
| return foundSoFar; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here's the problem that was reported as #4229: we are to collect ALL matches, not just main-level one
| foundSoFar = new ArrayList<>(); | ||
| } | ||
| foundSoFar.add(jsonNode.asText()); | ||
| return foundSoFar; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and here
ObjectNode::findValue(String)andObjectNode::findParent(String)currently traverse the node's children entries looking for a matchingpropertyName, leading toO(n)access time per level in anObjectNodegraph.Both of these methods could be optimized to perform a
O(1)LinkedHashMap.get(String)lookup for the fast path where givenObjectNodecontains a child property with thatpropertyName, before falling back to the slow path traversing all child values.