Remove redundant recordPromotedName calls - #3117
Merged
eamonnmcmanus merged 2 commits intoSep 12, 2026
Merged
Conversation
Marcono1234
commented
Sep 11, 2026
| reader.beginObject(); | ||
| JsonReaderInternalAccess.INSTANCE.promoteNameToValue(reader); | ||
| reader.skipValue(); | ||
| String expectedPath = factory == Factory.STRING_READER ? "$.<skipped>" : "$.name"; |
Contributor
Author
There was a problem hiding this comment.
This also highlights a difference between JsonReader and JsonTreeReader. But I guess this is fine: JsonReader uses the dummy name <skipped> for efficiency to avoid having to obtain the name as String. Whereas JsonTreeReader has the name available as String anyway.
For JsonTreeReader the behavior for skipValue() now differs between a regular name and a promoted one, but that is probably still fine.
Trying to fix that would make the JsonTreeReader implementation more complex because it would now need a dedicated field as well to remember whether a name had been promoted.
Marcono1234
marked this pull request as ready for review
September 11, 2026 11:50
These calls are guarded by a `case PEEKED_LONG`, but it seems `PEEKED_LONG` is only used if an actual number occurred in the JSON document. Whereas when promoting a name to a value, it will always be a JSON string value. These redundant calls were also the "expensive" ones, converting a long to a String.
Marcono1234
force-pushed
the
marcono1234/redundant-promoted-name
branch
from
September 11, 2026 11:56
ee090fc to
1c328fc
Compare
eamonnmcmanus
approved these changes
Sep 12, 2026
eamonnmcmanus
left a comment
eamonnmcmanus
left a comment
Member
There was a problem hiding this comment.
Thanks for noticing this!
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
2 participants
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
Follow-up for #3098
Description
These
recordPromotedNamecalls are guarded by acase PEEKED_LONG, but it seemsPEEKED_LONGis only used if an actual number occurred in the JSON document. Whereas when promoting a name to a value, it will always be a JSON string value.These redundant calls were also the "expensive" ones, converting a long to a String.