optimize resource table build - #104
Open
rotundtapir wants to merge 4 commits into
Open
Conversation
BlockList.sort() registered a swap listener that called setIndex() on both items of every quicksort swap. For items that maintain back references, such as StringItem, each setIndex() triggers onIndexChanged() which rewrites every reference to the item, so a single sort of a large string pool performed O(swaps x references) work while all intermediate index values were immediately overwritten. The updateIndex() pass that already runs after the sort assigns each item its final index exactly once, firing onIndexChanged() a single time per moved item, so the per-swap updates are redundant. Sorting a resource table string pool of a large apk gets several times faster; the resulting order and all reference values are unchanged.
BlockList.sort() ran the full quicksort even when the list was already sorted, costing O(n log n) comparisons per call. The string pools re-sort on every refresh whenever the sort-required flag is set, and operations that preserve order, like removing unused strings, set that flag too, so a resource table refreshed several times during encoding paid for the same sort repeatedly. Use the existing needsSort() linear scan to detect the already-sorted case and return false, matching the previous return value since the quicksort performs no swaps on a sorted list. needsSort() also covers the size() < 2 check. A genuinely unsorted list pays one extra O(n) scan, which is small against the O(n log n) sort that follows.
encodeValues() refreshed every package block right after encoding it, which recomputes sizes, offsets and header fields of the whole package, and re-sorts its string pools. All of that is thrown away: the table is refreshed at the end of scanResourceFiles() and again by ApkModuleEncoder.refreshTable(), and nothing between the per-package refresh and those passes reads the computed sizes or offsets. Keep sortTypes(), which later steps do rely on, and let the table-level refresh do the byte-level bookkeeping once.
TypeBlock.getOrCreateDefinedEntry() started with getEntry(name), which walks every entry of the whole package referencing the spec string with that name and tests each for membership in this type block. While encoding values XML the lookup almost always misses, because the current type block is the one being filled, so each encoded item paid for a walk over all previously encoded configurations of the same resource: quadratic in the number of configurations, and the dominant cost of building a resource table with many locales. The subsequent resolveResourceId() walked the same references again. Resolve the id first (the reference walk stops at the first entry of this type), then address the entry array directly by entry id and compare the name. The scan by name is kept only for the ambiguous case where the defined id already holds a different name. Behavior is unchanged: a defined entry is returned when present, created at its defined id when not, and an undefined name still returns null.
Owner
|
Your results are impressive ! |
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.
Four changes to the resource table build path. Output is unchanged; the win is in avoided repeated work.
BlockList.sort: update block indexes once after sorting instead of on every swap, and skip sorting a list that is already in order.XMLTableBlockEncoder.encodeResDir: drop the per-packagerefresh(), which the finaltableBlock.refresh()already covers.TypeBlock.getOrCreateDefinedEntry: look up the entry by resolved id instead of scanning the entry references.Encoding time for a decode then re-encode, mean of 5 runs per side, alternating, fresh JVM each run, JDK 17:
The gain scales with table size, which is what you would expect from removing the per-swap index updates and the entry scan. On the smallest table the difference is inside the run-to-run spread, so I would not claim one there.
The produced
resources.arscis byte-identical tomainfor all three, same SHA-256 and same length, starting from the same decoded directory. For YouTube that is 20331984 bytes /865be6cb389785dd...on both sides.Related: #97 was the same shape of problem in the decode path.
One note while I was in there: calling
TableBlock.refresh()an extra time before serialising changes the output bytes while keeping the length identical, sorefresh()is not byte-idempotent. Nothing here depends on that, but it made verifying output identity fiddlier than expected and might be worth a look separately.Written with AI assistance (Claude Code); the timings and the byte-identity check are reproducible with the harness described above, and I reviewed everything before submitting.