Read release file sizes from the archive instead of extracting it - #76
Open
dionmcm wants to merge 1 commit into
Open
Read release file sizes from the archive instead of extracting it#76dionmcm wants to merge 1 commit into
dionmcm wants to merge 1 commit into
Conversation
The snapshot and full file-size tests extracted four whole release directories - prospective and previous, snapshot and full - through ReleaseImporter, and then called File.length() on the results. The uncompressed size of every entry is in the ZIP central directory already. On a real 853MB edition that extraction was around 100 seconds, and it happens only when a previous release is supplied, so it is invisible to a first-time-release run and unavoidable for a nightly that compares against last month's release. ZipEntry.getSize() is the uncompressed size and is exact: checked against a full extraction of that release, 76 entries, zero mismatches and none returning the unknown -1. Where an entry does return -1 - a zip written as a stream, with sizes only in the trailing data descriptor - it is excluded with a warning rather than compared against a guess, because a wrong size here is a wrong finding about a release. Threading the extraction was the other option and it is not worth having: ZipFileUtils.extractFilesFromZipToOneFolder already parallelises, extracts that edition in 4.5s at 1223 MB/s, and a hand-rolled largest-entry-first pool only reached 4.2s. The cost was never the unzip that was needed, it was the four that were not. Behaviour preserved: .txt entries only, paired on everything before the last underscore so the date suffix is dropped, first match wins, same messages and same KB rounding. One visible difference - the error's filePath was the temporary extraction directory, which was deleted in a finally block before any reader could reach it, and is now the archive path. Ten tests. Two mutations were checked rather than assumed: removing the release-type filter fails two of them, so Snapshot and Full cannot silently start comparing each other's files.
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
1 participant
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.
The snapshot and full file-size tests extracted four whole release directories — prospective and previous, snapshot and full — through
ReleaseImporter, then calledFile.length()on the results. The uncompressed size of every entry is already in the ZIP central directory.On a real 853 MB edition that extraction is about 100 seconds. It only runs when a previous release is supplied, so a first-time-release run never sees it, and a nightly comparing against last month's release always pays the cost.
Measured
Same release and request, 8 cores, before and after:
getSize()Checked against a full extraction of a release, 76 entries, zero mismatches, none returning the unknown-1.Behaviour preserved
.txtentries only; paired on everything before the last underscore so the date suffix is dropped; first match wins; same messages and same KB rounding.One difference - the error's
filePathused to be the temporary extraction directory, which was deleted in afinallyblock before any reader could reach it. It is now the archive path.Tests
Ten, over real zips built in the test. Two mutations were checked
onlyTheRequestedReleaseTypeIsReturnedandanEmptyReleaseTypeYieldsNothingRatherThanFailing, so Snapshot and Full cannot silently begin comparing each other's files;aSmallerProspectiveFileIsStillReportedandanEqualOrLargerProspectiveFilePassesensure that the rule still fires