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

Optimize loops #341

Merged
merged 3 commits into from
Jun 1, 2017
Merged

Optimize loops #341

merged 3 commits into from
Jun 1, 2017

Conversation

johnjaylward
Copy link
Contributor

@johnjaylward johnjaylward commented May 22, 2017

Changes for #340

  • Adds protected entrySet accessor to JSONObject
  • Updates loops that request key/value pairs to use the new entrySet accessor

What problem does this code solve?
This is a performance enhancement, not a bug fix. Some code is refactored to use new loop types and map iterators introduced in Java5. One assignment (line 541) was deleted from the XML class as it didn't do anything.

Risks
low

Changes to the API?
New protected method added for internal access to the backing map entrySet. I wasn't comfortable making this a public method without input from the community as it allows unfiltered access to the entire map (keys and values).

Will this require a new release?
No, can be rolled into the next release

Should the documentation be updated?
No

Does it break the unit tests?
No. Also, no unit tests have been added at this time.

Was any code refactored in this commit?
Yes

Review status
ACCEPTED. A few lines of new code in JSONArray and JSONObject are not tested, but this can be picked up in the next test refactoring. Starting 3 day timer for comments.

* Updates loops that request key/value pairs to use the new entrySet accessor
@johnjaylward
Copy link
Contributor Author

johnjaylward commented May 23, 2017

updated the branch to the latest master

@johnjaylward
Copy link
Contributor Author

@TheMatthew please review when convenient

@TheMatthew
Copy link

I will asap, thanks @johnjaylward

@TheMatthew
Copy link

The performance gain I am seeing is quite good. it is passing from 125 seconds to 115 seconds for a 2.1gb file. it is even more interesting if the file is already in ram, it goes from ~98 seconds to 85.

@stleary
Copy link
Owner

stleary commented May 24, 2017

Looks good, comment timer started.

@stleary stleary merged commit ef7a5e4 into stleary:master Jun 1, 2017
@johnjaylward johnjaylward deleted the OptimizeLoops branch July 7, 2017 16:34
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

Successfully merging this pull request may close these issues.

3 participants