From 6b705a71a31722a6dfdd29b17267c7f1170ed8ed Mon Sep 17 00:00:00 2001 From: rotund_tapir Date: Thu, 27 Aug 2026 09:57:45 +0100 Subject: [PATCH 1/8] Update block indexes once after sorting instead of on every swap 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. --- .../java/com/reandroid/arsc/container/BlockList.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/main/java/com/reandroid/arsc/container/BlockList.java b/src/main/java/com/reandroid/arsc/container/BlockList.java index cff7a2be7..2b85ffca3 100755 --- a/src/main/java/com/reandroid/arsc/container/BlockList.java +++ b/src/main/java/com/reandroid/arsc/container/BlockList.java @@ -201,16 +201,7 @@ public boolean sort(Comparator comparator) { if (size() < 2) { return false; } - boolean sorted = mItems.sort(comparator, (i, j) -> { - T item1 = get(i); - T item2 = get(j); - if (item1 != null) { - item1.setIndex(i); - } - if (item2 != null) { - item2.setIndex(j); - } - }); + boolean sorted = mItems.sortItems(comparator); if (sorted) { updateIndex(); } From 58fe24419d41c06272dd4c4f23d7b19906c95fe5 Mon Sep 17 00:00:00 2001 From: rotund_tapir Date: Thu, 27 Aug 2026 09:57:57 +0100 Subject: [PATCH 2/8] Skip sorting a block list that is already in order 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. --- src/main/java/com/reandroid/arsc/container/BlockList.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/reandroid/arsc/container/BlockList.java b/src/main/java/com/reandroid/arsc/container/BlockList.java index 2b85ffca3..9a89910e6 100755 --- a/src/main/java/com/reandroid/arsc/container/BlockList.java +++ b/src/main/java/com/reandroid/arsc/container/BlockList.java @@ -198,7 +198,7 @@ public void destroy() { onChanged(); } public boolean sort(Comparator comparator) { - if (size() < 2) { + if (!needsSort(comparator)) { return false; } boolean sorted = mItems.sortItems(comparator); From 8f5ea50d092406668e5992e170f8684f676369e6 Mon Sep 17 00:00:00 2001 From: rotund_tapir Date: Thu, 27 Aug 2026 10:01:07 +0100 Subject: [PATCH 3/8] Drop the per-package refresh while encoding values 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. --- .../java/com/reandroid/apk/xmlencoder/XMLTableBlockEncoder.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/reandroid/apk/xmlencoder/XMLTableBlockEncoder.java b/src/main/java/com/reandroid/apk/xmlencoder/XMLTableBlockEncoder.java index afcb4fa6d..67d8b4eb4 100644 --- a/src/main/java/com/reandroid/apk/xmlencoder/XMLTableBlockEncoder.java +++ b/src/main/java/com/reandroid/apk/xmlencoder/XMLTableBlockEncoder.java @@ -187,7 +187,6 @@ private void encodeValues(List pubXmlFileList) throws IOException, XmlPull filePathEncoder.encodePackageResDir(packageBlock, resDir); packageBlock.sortTypes(); - packageBlock.refresh(); } } private void encodeAttrs(List pubXmlFileList) throws IOException, XmlPullParserException { From 0be2f07efe8567f969b89ece9453cda8cf435cc7 Mon Sep 17 00:00:00 2001 From: rotund_tapir Date: Thu, 27 Aug 2026 10:07:15 +0100 Subject: [PATCH 4/8] Look up defined entries by resolved id instead of scanning references 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. --- .../com/reandroid/arsc/chunk/TypeBlock.java | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/reandroid/arsc/chunk/TypeBlock.java b/src/main/java/com/reandroid/arsc/chunk/TypeBlock.java index b5431d0dd..a64f01c25 100755 --- a/src/main/java/com/reandroid/arsc/chunk/TypeBlock.java +++ b/src/main/java/com/reandroid/arsc/chunk/TypeBlock.java @@ -186,10 +186,6 @@ public SpecTypePair getParentSpecTypePair() { return getParent(SpecTypePair.class); } public Entry getOrCreateDefinedEntry(String name) { - Entry entry = getEntry(name); - if (entry != null) { - return entry; - } PackageBlock packageBlock = getPackageBlock(); if (packageBlock == null) { return null; @@ -198,6 +194,21 @@ public Entry getOrCreateDefinedEntry(String name) { if (id == 0) { return null; } + Entry entry = getEntry(id & 0xffff); + if (entry != null) { + String entryName = entry.getName(); + if (name.equals(entryName)) { + return entry; + } + if (entryName != null && !entry.isNull()) { + // The defined id holds a different name; fall back to + // scanning this type block by name. + Entry existing = getEntry(name); + if (existing != null) { + return existing; + } + } + } SpecStringPool stringPool = packageBlock.getSpecStringPool(); SpecString specString = stringPool.getOrCreate(name); entry = getOrCreateEntry(id & 0xffff); From 0a594c477f869360f56e221af9c7b243b6de3eb3 Mon Sep 17 00:00:00 2001 From: rotund_tapir Date: Sun, 27 Sep 2026 14:45:10 +0100 Subject: [PATCH 5/8] Grow ArrayCollection geometrically when inserting at an index add(index, item) went through slideRight(), which reserved exactly the missing slot, so every insertion reallocated and copied the whole array. Building a type block entry by entry, as merging tables does, was quadratic because of it. Also fix ensureCapacity(int) allocating too little when some capacity is already free: it subtracted the free slots from the request and then added the result to the size instead of to the current length, which could leave fewer free slots than asked for. --- .../utils/collection/ArrayCollection.java | 12 +++++-- .../utils/collection/ArrayCollectionTest.java | 35 +++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 src/test/java/com/reandroid/utils/collection/ArrayCollectionTest.java diff --git a/src/main/java/com/reandroid/utils/collection/ArrayCollection.java b/src/main/java/com/reandroid/utils/collection/ArrayCollection.java index 558adb0a3..b7ec5f44b 100644 --- a/src/main/java/com/reandroid/utils/collection/ArrayCollection.java +++ b/src/main/java/com/reandroid/utils/collection/ArrayCollection.java @@ -1026,7 +1026,12 @@ private void slideLeft(int position, int amount){ private void slideRight(int position, int amount){ boolean locked = mLocked; this.mLocked = true; - ensureCapacity(amount); + if(availableCapacity() < amount){ + // Grow by the usual step rather than by exactly 'amount', otherwise + // inserting n items one by one reallocates and copies the array n times + int grow = calculateGrow(); + ensureCapacity(grow > amount ? grow : amount); + } Object[] elements = this.mElements; int size = this.size; int i = size - 1; @@ -1095,9 +1100,10 @@ public void ensureCapacity(int capacity) { return; } int size = this.size; - int length = size + capacity; - Object[] update = getNewArray(length); Object[] elements = this.mElements; + // 'capacity' is what is still missing on top of the free slots + int length = elements.length + capacity; + Object[] update = getNewArray(length); if(elements.length == 0 || size == 0){ this.mElements = update; return; diff --git a/src/test/java/com/reandroid/utils/collection/ArrayCollectionTest.java b/src/test/java/com/reandroid/utils/collection/ArrayCollectionTest.java new file mode 100644 index 000000000..75649e6f7 --- /dev/null +++ b/src/test/java/com/reandroid/utils/collection/ArrayCollectionTest.java @@ -0,0 +1,35 @@ +package com.reandroid.utils.collection; + +import org.junit.Assert; +import org.junit.Test; + +public class ArrayCollectionTest { + + @Test + public void testInsertAtIndexThenSetSize() { + ArrayCollection collection = new ArrayCollection<>(); + for (int i = 0; i < 1000; i++) { + collection.add(i / 2, i); + } + Assert.assertEquals(1000, collection.size()); + // insertions leave spare capacity behind, growing must still fit + collection.setSize(1500); + Assert.assertEquals(1500, collection.size()); + collection.ensureCapacity(10); + Assert.assertTrue(collection.availableCapacity() >= 10); + int free = collection.availableCapacity(); + collection.ensureCapacity(free + 5); + Assert.assertTrue(collection.availableCapacity() >= free + 5); + } + + @Test + public void testInsertOrder() { + ArrayCollection collection = new ArrayCollection<>(); + for (int i = 0; i < 100; i++) { + collection.add(0, i); + } + for (int i = 0; i < 100; i++) { + Assert.assertEquals(Integer.valueOf(99 - i), collection.get(i)); + } + } +} From 035a7c1107fb3e4b4df84041d365f7790313c913 Mon Sep 17 00:00:00 2001 From: rotund_tapir Date: Sun, 27 Sep 2026 14:52:39 +0100 Subject: [PATCH 6/8] Merge bundle tables without copying the base table ApkBundle.mergeModules() merged the base into an empty module, which copied the whole base table entry by entry into a new TableBlock while the base kept its own parsed copy, and TableBlock.merge() refreshed the whole table after every module. On a large app (Google app, 87 MB resources.arsc) that is two full tables on the heap and one full refresh per split. A table that is only parsed for the merge is now taken over as the merged table when the target has none, a split's parsed table is dropped once merged, and TableBlock.merge(table, refresh) lets the bundle refresh once at the end. The public merge() methods still refresh. The largest table is picked by entry size, so a bundle without a base does not parse every table up front. Taking the base table over also keeps its strings as they are: the copy merged the string pool by XML text, which could turn a plain string containing markup into the styled string of the same text. Also keep a stored dex stored under the name it is renumbered to, instead of deflating it while writing. --- .../java/com/reandroid/apk/ApkBundle.java | 18 ++-- .../java/com/reandroid/apk/ApkModule.java | 67 ++++++++++--- .../com/reandroid/arsc/chunk/TableBlock.java | 11 ++- .../com/reandroid/apk/ApkBundleMergeTest.java | 94 +++++++++++++++++++ 4 files changed, 168 insertions(+), 22 deletions(-) create mode 100644 src/test/java/com/reandroid/apk/ApkBundleMergeTest.java diff --git a/src/main/java/com/reandroid/apk/ApkBundle.java b/src/main/java/com/reandroid/apk/ApkBundle.java index f8753ac52..fb4b725e4 100644 --- a/src/main/java/com/reandroid/apk/ApkBundle.java +++ b/src/main/java/com/reandroid/apk/ApkBundle.java @@ -51,7 +51,7 @@ public ApkModule mergeModules(boolean force) throws IOException { if(base == null){ base = getLargestTableModule(); } - result.merge(base, force); + result.merge(base, force, false); AndroidManifestBlockMerger manifestMerger = getManifestMerger(); if (manifestMerger != null) { manifestMerger.reset(); @@ -69,7 +69,7 @@ public ApkModule mergeModules(boolean force) throws IOException { if(signatureBlock == null){ signatureBlock = asb; } - result.merge(module, force); + result.merge(module, force, false); if (manifestMerger != null) { manifestMerger.merge(module.getAndroidManifest()); } @@ -107,17 +107,19 @@ private String generateMergedModuleName(){ } return name; } - private ApkModule getLargestTableModule(){ + private ApkModule getLargestTableModule() throws IOException { + // Compared by entry size, so the tables are not all parsed just to pick one ApkModule apkModule=null; - int chunkSize=0; + long largest=0; for(ApkModule module:getApkModuleList()){ if(!module.hasTableBlock()){ continue; } - TableBlock tableBlock=module.getTableBlock(); - int size=tableBlock.getHeaderBlock().getChunkSize(); - if(apkModule==null || size>chunkSize){ - chunkSize=size; + TableBlock loaded=module.getLoadedTableBlock(); + long size=loaded != null ? loaded.getHeaderBlock().getChunkSize() + : module.getInputSource(TableBlock.FILE_NAME).getLength(); + if(apkModule==null || size>largest){ + largest=size; apkModule=module; } } diff --git a/src/main/java/com/reandroid/apk/ApkModule.java b/src/main/java/com/reandroid/apk/ApkModule.java index 084c78d2d..0fcb3f6dc 100644 --- a/src/main/java/com/reandroid/apk/ApkModule.java +++ b/src/main/java/com/reandroid/apk/ApkModule.java @@ -1154,9 +1154,7 @@ TableBlock loadTableBlock() throws IOException { tableBlock = (TableBlock) ((BlockInputSource) inputSource).getBlock(); } else { setTableOriginalSource(inputSource); - InputStream inputStream = inputSource.openStream(); - tableBlock = TableBlock.load(inputStream); - inputStream.close(); + tableBlock = parseTableBlock(inputSource); } BlockInputSource blockInputSource = new BlockInputSource<>( inputSource.getName(), tableBlock); @@ -1220,13 +1218,20 @@ public void merge(ApkModule module) throws IOException { merge(module, false); } public void merge(ApkModule module, boolean force) throws IOException { + merge(module, force, true); + } + /** + * @param refreshTable whether to refresh the merged table afterwards. {@link ApkBundle} + * merges every module first and refreshes the table once at the end. + */ + void merge(ApkModule module, boolean force, boolean refreshTable) throws IOException { if (module == null || module == this) { return; } logMessage("Merging: " + module.getModuleName()); validateMerge(module, force); mergeDexFiles(module); - mergeTable(module); + mergeTable(module, refreshTable); mergeFiles(module); getUncompressedFiles().merge(module.getUncompressedFiles()); mergeFusedModules(module); @@ -1291,20 +1296,50 @@ private void validateMerge(ApkModule apkModule, boolean force) throws IOExceptio } logMessage(msg); } - private void mergeTable(ApkModule module) { + private void mergeTable(ApkModule module, boolean refresh) throws IOException { if (!module.hasTableBlock()) { return; } + TableBlock coming = module.getLoadedTableBlock(); + if (coming == null) { + // Parsed for this merge only, so nothing else holds it: it can become the merged + // table as is, or be dropped once merged + coming = module.loadDetachedTableBlock(); + if (coming != null && !hasTableBlock()) { + addInputSource(new BlockInputSource<>(TableBlock.FILE_NAME, coming)); + return; + } + if (coming == null) { + coming = module.getTableBlock(); + } + } TableBlock exist; if (!hasTableBlock()) { - exist=new TableBlock(); - BlockInputSource inputSource=new BlockInputSource<>(TableBlock.FILE_NAME, exist); - addInputSource(inputSource); + exist = new TableBlock(); + addInputSource(new BlockInputSource<>(TableBlock.FILE_NAME, exist)); } else { - exist=getTableBlock(); + exist = getTableBlock(); + } + exist.merge(coming, refresh); + } + /** + * Parses resources.arsc without caching it on this module, or returns null when the + * table source is already a block rather than an archive entry. + */ + private TableBlock loadDetachedTableBlock() throws IOException { + InputSource inputSource = getInputSource(TableBlock.FILE_NAME); + if (inputSource == null || inputSource instanceof BlockInputSource) { + return null; + } + return parseTableBlock(inputSource); + } + private static TableBlock parseTableBlock(InputSource inputSource) throws IOException { + InputStream inputStream = inputSource.openStream(); + try { + return TableBlock.load(inputStream); + } finally { + inputStream.close(); } - TableBlock coming=module.getTableBlock(); - exist.merge(coming); } private void mergeFiles(ApkModule module) { ZipEntryMap entryMapExist = getZipEntryMap(); @@ -1328,7 +1363,8 @@ private void mergeFiles(ApkModule module) { } } private void mergeDexFiles(ApkModule module) { - UncompressedFiles uncompressedFiles = module.getUncompressedFiles(); + UncompressedFiles comingUncompressed = module.getUncompressedFiles(); + UncompressedFiles uncompressed = getUncompressedFiles(); List existList = listDexFiles(); List comingList = module.listDexFiles(); ZipEntryMap zipEntryMap = getZipEntryMap(); @@ -1342,8 +1378,13 @@ private void mergeDexFiles(ApkModule module) { } } for (DexFileInputSource source : comingList) { - uncompressedFiles.removePath(source.getAlias()); String name = DexFileInputSource.getDexName(index); + if (comingUncompressed.isUncompressed(source.getAlias())) { + // Keep a stored dex stored under its new name, so it is copied as is + // instead of being deflated while writing + uncompressed.addPath(name); + } + comingUncompressed.removePath(source.getAlias()); DexFileInputSource add = new DexFileInputSource(name, source.getInputSource()); zipEntryMap.add(add); logMessage("Added [" + module.getModuleName() +"] " diff --git a/src/main/java/com/reandroid/arsc/chunk/TableBlock.java b/src/main/java/com/reandroid/arsc/chunk/TableBlock.java index 5bd4aa4a9..5ed1d262a 100755 --- a/src/main/java/com/reandroid/arsc/chunk/TableBlock.java +++ b/src/main/java/com/reandroid/arsc/chunk/TableBlock.java @@ -795,12 +795,21 @@ public void fromJson(JSONObject json) { refresh(); } public void merge(TableBlock tableBlock){ + merge(tableBlock, true); + } + /** + * @param refresh whether to refresh this table afterwards. A caller merging several tables + * can skip it for all but the last, as refreshing a large table is costly. + */ + public void merge(TableBlock tableBlock, boolean refresh){ if(tableBlock == null || tableBlock == this){ return; } getStringPool().merge(tableBlock.getStringPool()); getPackageArray().merge(tableBlock.getPackageArray()); - refresh(); + if(refresh){ + refresh(); + } } @Override public byte[] getBytes(){ diff --git a/src/test/java/com/reandroid/apk/ApkBundleMergeTest.java b/src/test/java/com/reandroid/apk/ApkBundleMergeTest.java new file mode 100644 index 000000000..c39489ab5 --- /dev/null +++ b/src/test/java/com/reandroid/apk/ApkBundleMergeTest.java @@ -0,0 +1,94 @@ +package com.reandroid.apk; + +import com.reandroid.TestUtils; +import com.reandroid.archive.ByteInputSource; +import com.reandroid.archive.ZipEntryMap; +import com.reandroid.arsc.chunk.PackageBlock; +import com.reandroid.arsc.chunk.TableBlock; +import com.reandroid.arsc.chunk.xml.AndroidManifestBlock; +import com.reandroid.arsc.value.Entry; +import org.junit.Assert; +import org.junit.Test; + +import java.io.File; +import java.io.IOException; + +public class ApkBundleMergeTest { + + private static final String PACKAGE = "com.reandroid.merge"; + + @Test + public void testMergeKeepsBaseResourcesAndAddsSplitConfigs() throws IOException { + ApkModule merged = merge(); + + PackageBlock packageBlock = merged.getTableBlock().pickOne(); + Assert.assertEquals("Hello", value(packageBlock, "", "hello")); + Assert.assertEquals("Hallo", value(packageBlock, "-de", "hello")); + Assert.assertEquals("Only in base", value(packageBlock, "", "base_only")); + } + + @Test + public void testMergeKeepsStoredDexStored() throws IOException { + ApkModule merged = merge(); + + Assert.assertTrue(merged.getUncompressedFiles().isUncompressed("classes.dex")); + Assert.assertTrue(merged.getUncompressedFiles().isUncompressed("classes2.dex")); + } + + /** Merges, writes and reloads the result, as callers use it */ + private static ApkModule merge() throws IOException { + ApkBundle bundle = new ApkBundle(); + bundle.addModule(createBase()); + bundle.addModule(createSplit()); + ApkModule merged = bundle.mergeModules(); + File file = new File(TestUtils.getTempDir(), "bundle_merged.apk"); + merged.writeApk(file); + merged.close(); + bundle.close(); + return ApkModule.loadApkFile(file); + } + + private static ApkModule createBase() throws IOException { + TableBlock tableBlock = new TableBlock(); + PackageBlock packageBlock = tableBlock.newPackage(0x7f, PACKAGE); + packageBlock.getOrCreate("", "string", "hello").setValueAsString("Hello"); + packageBlock.getOrCreate("", "string", "base_only").setValueAsString("Only in base"); + tableBlock.refreshFull(); + return createModule("base", tableBlock, null); + } + + private static ApkModule createSplit() throws IOException { + TableBlock tableBlock = new TableBlock(); + PackageBlock packageBlock = tableBlock.newPackage(0x7f, PACKAGE); + // Created first, so it gets the id "hello" has in the base + packageBlock.getOrCreate("-de", "string", "hello").setValueAsString("Hallo"); + tableBlock.refreshFull(); + return createModule("split_config.de", tableBlock, "config.de"); + } + + private static ApkModule createModule(String name, TableBlock tableBlock, String split) { + ApkModule module = new ApkModule(name, new ZipEntryMap()); + module.setLoadDefaultFramework(false); + + AndroidManifestBlock manifest = new AndroidManifestBlock(); + manifest.setPackageName(PACKAGE); + if (split != null) { + manifest.setSplit(split, true); + } + manifest.refresh(); + module.add(new ByteInputSource(manifest.getBytes(), AndroidManifestBlock.FILE_NAME)); + + // A table the module has not parsed yet, as when loaded from an archive + module.add(new ByteInputSource(tableBlock.getBytes(), TableBlock.FILE_NAME)); + + module.add(new ByteInputSource(new byte[]{'d', 'e', 'x', '\n'}, "classes.dex")); + module.getUncompressedFiles().addPath("classes.dex"); + return module; + } + + private static String value(PackageBlock packageBlock, String qualifiers, String name) { + Entry entry = packageBlock.getEntry(qualifiers, "string", name); + Assert.assertNotNull(qualifiers + "/" + name, entry); + return entry.getResValue().getValueAsString(); + } +} From ec009d2ecfad6c635e530b9bbee88dd6c5485c6a Mon Sep 17 00:00:00 2001 From: rotund_tapir Date: Sun, 27 Sep 2026 14:53:31 +0100 Subject: [PATCH 7/8] Use an open addressing set for string references Every string and spec string keeps its references in a HashSetStore, which became a java.util.HashSet once it held more than one. A large table has millions of references, so the 32 byte HashMap node per element added up (about 130 MB on an 87 MB resources.arsc). The set is now an open addressing table with linear probing and backward shift deletion, costing one array slot per element, with the same equals/hashCode semantics. --- .../com/reandroid/utils/HashSetStore.java | 185 ++++++++++++++++-- .../com/reandroid/utils/HashSetStoreTest.java | 81 ++++++++ 2 files changed, 254 insertions(+), 12 deletions(-) create mode 100644 src/test/java/com/reandroid/utils/HashSetStoreTest.java diff --git a/src/main/java/com/reandroid/utils/HashSetStore.java b/src/main/java/com/reandroid/utils/HashSetStore.java index 7cabb8dc6..94f9eedf2 100644 --- a/src/main/java/com/reandroid/utils/HashSetStore.java +++ b/src/main/java/com/reandroid/utils/HashSetStore.java @@ -21,9 +21,11 @@ import com.reandroid.utils.collection.InstanceIterator; import com.reandroid.utils.collection.SingleIterator; +import java.util.AbstractSet; +import java.util.Arrays; import java.util.Collection; -import java.util.HashSet; import java.util.Iterator; +import java.util.NoSuchElementException; import java.util.function.Predicate; @@ -31,7 +33,7 @@ * A utility class to hold few non-null objects, the main purpose is to minimize new HashSet class creation. *
If no entries, the container will be null. *
If one entry, the container will become entry itself. - *
If more than one entry, HashSet class will be created and becomes the container. + *
If more than one entry, a compact hash set is created and becomes the container. * */ public class HashSetStore { @@ -267,36 +269,195 @@ public static Object create(Iterator iterator) { return set; } - static final class ObjectsSet extends HashSet { + /** + * Open addressing (linear probing) hash set. The containers here hold the + * references of every string / spec string of a resource table, so per element + * overhead matters: a HashSet costs a 32 byte node per element plus its table, + * this costs one array slot. + */ + static final class ObjectsSet extends AbstractSet { + private static final int MIN_CAPACITY = 4; + + private Object[] table; + private int size; ObjectsSet() { - super(); + this.table = new Object[MIN_CAPACITY]; } ObjectsSet(Object[] elements) { - super(elements.length); - for (Object obj : elements) { - add(obj); - } + this.table = new Object[capacityFor(elements.length)]; + addAll(elements); } ObjectsSet(Collection collection) { - super(collection); + this.table = new Object[capacityFor(collection.size())]; + addAll(collection); + } + + private static int capacityFor(int count) { + int capacity = MIN_CAPACITY; + // keep the load factor at or below 3/4 + while (capacity - (capacity >>> 2) < count) { + capacity = capacity << 1; + } + return capacity; + } + private static int indexFor(Object element, int mask) { + int h = element.hashCode() * 0x9E3779B9; + return (h ^ (h >>> 16)) & mask; + } + /** + * @return the slot holding element, or -(slot + 1) of the + * empty slot where it would go + */ + private int find(Object element) { + Object[] table = this.table; + int mask = table.length - 1; + int i = indexFor(element, mask); + Object existing; + while ((existing = table[i]) != null) { + if (existing == element || existing.equals(element)) { + return i; + } + i = (i + 1) & mask; + } + return -(i + 1); + } + + @Override + public int size() { + return size; + } + @Override + public boolean contains(Object o) { + return o != null && find(o) >= 0; + } + /** Null is ignored, as {@link HashSetStore} never holds it */ + @Override + public boolean add(Object element) { + if (element == null) { + return false; + } + int i = find(element); + if (i >= 0) { + return false; + } + Object[] table = this.table; + table[-i - 1] = element; + size ++; + if (size > table.length - (table.length >>> 2)) { + resize(table.length << 1); + } + return true; + } + private void resize(int capacity) { + Object[] old = this.table; + Object[] table = new Object[capacity]; + int mask = capacity - 1; + for (Object element : old) { + if (element != null) { + int i = indexFor(element, mask); + while (table[i] != null) { + i = (i + 1) & mask; + } + table[i] = element; + } + } + this.table = table; + } + @Override + public boolean remove(Object o) { + int i = o == null ? -1 : find(o); + if (i < 0) { + return false; + } + Object[] table = this.table; + int mask = table.length - 1; + table[i] = null; + size --; + // shift back the following run so lookups never hit a gap + int j = i; + while (true) { + j = (j + 1) & mask; + Object element = table[j]; + if (element == null) { + break; + } + int k = indexFor(element, mask); + boolean stays = (i <= j) ? (i < k && k <= j) : (i < k || k <= j); + if (!stays) { + table[i] = element; + table[j] = null; + i = j; + } + } + return true; } + @Override + public void clear() { + Arrays.fill(this.table, null); + this.size = 0; + } + @Override + public Object[] toArray() { + Object[] result = new Object[size]; + Object[] table = this.table; + int index = 0; + for (Object element : table) { + if (element != null) { + result[index++] = element; + } + } + return result; + } + @Override + public Iterator iterator() { + return new SlotIterator(table); + } + /** + * Yields the occupied slots. Not an {@link ArrayIterator}, which would report the + * table capacity as its size and make callers pre-size for empty slots. + */ + private static final class SlotIterator implements Iterator { + private final Object[] slots; + private int position; + SlotIterator(Object[] slots) { + this.slots = slots; + this.position = next(0); + } + private int next(int i) { + Object[] slots = this.slots; + while (i < slots.length && slots[i] == null) { + i ++; + } + return i; + } + @Override + public boolean hasNext() { + return position < slots.length; + } + @Override + public Object next() { + int i = this.position; + if (i >= slots.length) { + throw new NoSuchElementException(); + } + this.position = next(i + 1); + return slots[i]; + } + } public Iterator iterator(Class instance) { return ObjectsUtil.cast(InstanceIterator.of(iterator(), instance)); } - public Object getFirst() { if (!isEmpty()) { return iterator().next(); } return null; } - public Iterator clonedIterator() { return ArrayIterator.of(toArray()); } - public void addAll(Iterator iterator) { while (iterator.hasNext()) { add(iterator.next()); diff --git a/src/test/java/com/reandroid/utils/HashSetStoreTest.java b/src/test/java/com/reandroid/utils/HashSetStoreTest.java new file mode 100644 index 000000000..141e23fd2 --- /dev/null +++ b/src/test/java/com/reandroid/utils/HashSetStoreTest.java @@ -0,0 +1,81 @@ +package com.reandroid.utils; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.HashSet; +import java.util.Iterator; +import java.util.Random; +import java.util.Set; + +public class HashSetStoreTest { + + @Test + public void testRandomAddRemoveMatchesHashSet() { + Random random = new Random(7); + Object[] pool = new Object[300]; + for (int i = 0; i < pool.length; i++) { + // few distinct hash codes to force long probe runs + final int id = i; + pool[i] = new Object() { + @Override + public int hashCode() { + return id % 13; + } + }; + } + Object container = null; + Set expected = new HashSet<>(); + for (int round = 0; round < 20000; round++) { + Object item = pool[random.nextInt(pool.length)]; + if (random.nextInt(3) == 0) { + container = HashSetStore.remove(container, item); + expected.remove(item); + } else { + container = HashSetStore.add(container, item); + expected.add(item); + } + Assert.assertEquals(expected.size(), HashSetStore.size(container)); + if (round % 97 == 0) { + assertSameElements(expected, container); + } + } + assertSameElements(expected, container); + for (Object item : pool) { + Assert.assertEquals(expected.contains(item), HashSetStore.contains(container, item)); + } + } + + @Test + public void testSmallContainers() { + Object a = "a"; + Object b = "b"; + Object container = HashSetStore.add(null, a); + Assert.assertSame(a, container); + container = HashSetStore.add(container, b); + Assert.assertEquals(2, HashSetStore.size(container)); + container = HashSetStore.add(container, "b"); + Assert.assertEquals(2, HashSetStore.size(container)); + container = HashSetStore.remove(container, a); + Assert.assertSame(b, container); + container = HashSetStore.remove(container, b); + Assert.assertNull(container); + Assert.assertTrue(HashSetStore.isEmpty(container)); + } + + private static void assertSameElements(Set expected, Object container) { + Set actual = new HashSet<>(); + Iterator iterator = HashSetStore.iterator(container); + while (iterator.hasNext()) { + Assert.assertTrue("duplicate element", actual.add(iterator.next())); + } + Assert.assertEquals(expected, actual); + Iterator cloned = HashSetStore.clonedIterator(container); + int count = 0; + while (cloned.hasNext()) { + Assert.assertTrue(expected.contains(cloned.next())); + count++; + } + Assert.assertEquals(expected.size(), count); + } +} From c1d965d446ce0aba8a1d323a89fc20886b9e5ae4 Mon Sep 17 00:00:00 2001 From: rotund_tapir Date: Sun, 27 Sep 2026 14:53:31 +0100 Subject: [PATCH 8/8] Keep offset item values in fields instead of byte arrays There is an OffsetItem per entry and per string of every table, and each one carried its own 2 or 4 byte array next to the object. The offset (and index for sparse entries) now lives in int fields and is encoded when the item is written, which drops an array per item for every table load. The written bytes are unchanged. --- .../com/reandroid/arsc/item/OffsetItem.java | 176 +++++++++++++----- 1 file changed, 129 insertions(+), 47 deletions(-) diff --git a/src/main/java/com/reandroid/arsc/item/OffsetItem.java b/src/main/java/com/reandroid/arsc/item/OffsetItem.java index d055882e7..b6eea0327 100644 --- a/src/main/java/com/reandroid/arsc/item/OffsetItem.java +++ b/src/main/java/com/reandroid/arsc/item/OffsetItem.java @@ -24,6 +24,7 @@ import com.reandroid.utils.ObjectsUtil; import java.io.IOException; +import java.io.InputStream; public abstract class OffsetItem extends BlockItem implements DirectStreamReader, Comparable { @@ -35,13 +36,81 @@ public abstract class OffsetItem extends BlockItem implements DirectStreamReader public static final Creator CREATOR_OFFSET32 = Helper.init32(); public static final Creator CREATOR_SPARSE = Helper.initSparse(); + /* + * Offsets exist once per entry and per string of every table, so the value is kept + * only in fields and encoded on demand instead of backing each item with its own + * byte array. + */ private int mOffset; - protected OffsetItem(int bytesLength) { - super(bytesLength); - } protected OffsetItem() { - this(4); + super(0); + } + + /** The encoded size, 2 or 4 bytes */ + abstract int bytesLength(); + /** Encodes the current values into bytes of {@link #bytesLength()} */ + abstract void encode(byte[] bytes); + /** Decodes values from bytes of {@link #bytesLength()} */ + abstract void decode(byte[] bytes); + + private byte[] encode() { + byte[] bytes = new byte[bytesLength()]; + encode(bytes); + return bytes; + } + @Override + protected byte[] getBytesInternal() { + return encode(); + } + @Override + void setBytesInternal(byte[] bytes, boolean notify) { + if (bytes == null || bytes.length < bytesLength()) { + byte[] update = new byte[bytesLength()]; + if (bytes != null) { + System.arraycopy(bytes, 0, update, 0, bytes.length); + } + bytes = update; + } + decode(bytes); + } + @Override + int getBytesLength() { + return bytesLength(); + } + @Override + public int countBytes() { + if (isNull()) { + return 0; + } + return bytesLength(); + } + @Override + public void onReadBytes(BlockReader reader) throws IOException { + byte[] bytes = new byte[bytesLength()]; + reader.readFully(bytes); + decode(bytes); + } + @Override + public int readBytes(InputStream inputStream) throws IOException { + byte[] bytes = new byte[bytesLength()]; + int length = bytes.length; + int offset = 0; + int read = length; + while (length > 0 && read > 0) { + read = inputStream.read(bytes, offset, length); + length -= read; + offset += read; + } + decode(bytes); + super.notifyBlockLoad(); + return offset; + } + @Override + public void setBytes(BlockItem blockItem) { + if (blockItem != this) { + setBytesInternal(blockItem.getBytesInternal(), true); + } } public int getOffset() { @@ -49,10 +118,13 @@ public int getOffset() { } public void setOffset(int offset) { if (offset != mOffset) { - writeOffset(offset); + validateOffset(offset); mOffset = offset; } } + void setOffsetInternal(int offset) { + this.mOffset = offset; + } public int getIdx() { return getIndex(); @@ -60,13 +132,8 @@ public int getIdx() { public void setIdx(int idx) { } - protected abstract int readOffset(); - protected abstract void writeOffset(int offset); - - @Override - protected void onBytesChanged() { - super.onBytesChanged(); - this.mOffset = readOffset(); + /** Rejects a value that cannot be encoded, before it is set */ + protected void validateOffset(int offset) { } public boolean isNoEntry() { @@ -157,29 +224,39 @@ public String toString() { static class Offset16 extends OffsetItem { public Offset16() { - super(2); + super(); } @Override - protected int readOffset() { - int offset = getShortUnsigned(getBytesInternal(), 0); - if (offset == NO_ENTRY16) { - offset = NO_ENTRY; + int bytesLength() { + return 2; + } + @Override + void encode(byte[] bytes) { + int value = getOffset(); + if (value == NO_ENTRY) { + value = NO_ENTRY16; } else { - offset = offset * 4; + value = value / 4; } - return offset; + putShort(bytes, 0, value); } - @Override - protected void writeOffset(int offset) { - if (offset == NO_ENTRY) { - offset = NO_ENTRY16; + void decode(byte[] bytes) { + int value = getShortUnsigned(bytes, 0); + if (value == NO_ENTRY16) { + value = NO_ENTRY; } else { - offset = offset / 4; - validateValueRange(offset); + value = value * 4; + } + setOffsetInternal(value); + } + + @Override + protected void validateOffset(int offset) { + if (offset != NO_ENTRY) { + validateValueRange(offset / 4); } - putShort(getBytesInternal(), 0, offset); } @Override @@ -196,14 +273,18 @@ public Offset32() { } @Override - protected int readOffset() { - return getInteger(getBytesInternal(), 0); + int bytesLength() { + return 4; } - @Override - protected void writeOffset(int offset) { - putInteger(getBytesInternal(), 0, offset); + void encode(byte[] bytes) { + putInteger(bytes, 0, getOffset()); } + @Override + void decode(byte[] bytes) { + setOffsetInternal(getInteger(bytes, 0)); + } + @Override public int compareTo(OffsetItem offsetItem) { return compareIdx(offsetItem); @@ -218,6 +299,21 @@ public Sparse() { super(); } + @Override + int bytesLength() { + return 4; + } + @Override + void encode(byte[] bytes) { + putShort(bytes, 0, mIdx); + putShort(bytes, 2, getOffset() / 4); + } + @Override + void decode(byte[] bytes) { + this.mIdx = getShortUnsigned(bytes, 0); + setOffsetInternal(getShortUnsigned(bytes, 2) * 4); + } + @Override public int getIdx() { return mIdx; @@ -228,7 +324,6 @@ public void setIdx(int idx) { if (idx != mIdx) { validateValueRange(idx); this.mIdx = idx; - putShort(getBytesInternal(), 0, idx); } } @Override @@ -237,21 +332,8 @@ public boolean isNoEntry() { } @Override - protected void onBytesChanged() { - super.onBytesChanged(); - this.mIdx = getShortUnsigned(getBytesInternal(), 0); - } - - @Override - protected int readOffset() { - return getShortUnsigned(getBytesInternal(), 2) * 4; - } - - @Override - protected void writeOffset(int offset) { - int value = offset / 4; - validateValueRange(value); - putShort(getBytesInternal(), 2, value); + protected void validateOffset(int offset) { + validateValueRange(offset / 4); } @Override