Replace += with StringBuilder for whitespace to speed up Tokenizer #615
Conversation
This file contains 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
Previously the token buffer used StringBuilder but not the whitespace buffer - for large chunks of whitespace this resulted in very slow processing. This replaces concatenating charactres to `_wsString` with a StringBuilder
Codecov Report
@@ Coverage Diff @@
## integration #615 +/- ##
==============================================
Coverage 45.63% 45.63%
- Complexity 1046 1047 +1
==============================================
Files 57 57
Lines 9144 9146 +2
Branches 1682 1683 +1
==============================================
+ Hits 4173 4174 +1
Misses 4422 4422
- Partials 549 550 +1
Continue to review full report at Codecov.
|
carlwilson
approved these changes
Apr 6, 2022
carlwilson
added
feature
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.
This is a potential fix for issue #614 - whitespace is processed very slowly in the PDF Tokenizer. This became a problem when we had some files with >100MB of whitespace. Though the PDFs were not valid, JHOVE ran for days without getting to the end of the whitespace on a 160MB file. The fix uses a
StringBuilderinstead of doing+=on a String. It is now suspiciously fast, only taking a few seconds - which makes me wonder if I've missed something in the logic!Testing (edited): I have created a test file that can be used to replicate/test this issue and attached it to issue #614. I confirmed that this change reduces the processing time to seconds on both the original that we had the problem with (which I can't share) and my manufactured test file. I've submitted this change without a test for now - please let me know if I should add a test using my test file or if it needs to be added to a test corpus elsewhere.
Note: The issue that this relates to is newly logged and hasn't been evaluated yet - please let me know if it would be best to withdraw this PR until the issue is reviewed. My apologies if I've done things out of order!