Compress interface tables with packed encoding #121
Conversation
| throw new IllegalArgumentException(s"Interface $iface is not registed.") | ||
| ) | ||
| def getItableIdx(iface: IRNames.ClassName): Int = { | ||
| val idx = buckets.indexWhere(b => b.elements.contains(iface)) |
There was a problem hiding this comment.
This is linear in the number of interfaces in the program. Since it is called a linear amount of times (for each Apply to an interface method), this results in quadratic time overall.
Consider storing the itableIdx of an interface once and for all (during assignBuckets) in its ClassInfo, so that we can get it in O(1).
There was a problem hiding this comment.
Thanks! Right, as a consequence of storing the itableIdx to its ClassInfo (+ changes around start function), we no longer need to store the buckets in WasmContext 👍
| @@ -20,27 +20,28 @@ import wasm.ir2wasm.WasmExpressionBuilder | |||
| import org.scalajs.linker.interface.ModuleInitializer | |||
| import org.scalajs.linker.interface.unstable.ModuleInitializerImpl | |||
| import org.scalajs.linker.standard.LinkedTopLevelExport | |||
| import org.scalajs.linker.standard.LinkedClass | |||
|
|
|||
| abstract class ReadOnlyWasmContext { | |||
| import WasmContext._ | |||
|
|
|||
| protected val itableIdx = mutable.Map[IRNames.ClassName, Int]() | |||
There was a problem hiding this comment.
Looks like this became dead code.
| * The original paper set this to 255 so that a bucket index could fit in a single byte. | ||
| * However, maybe we can make it 65535 because we use i32 for indices anyway? | ||
| */ | ||
| val MAX_SIZE = 255 |
There was a problem hiding this comment.
I suggest we don't use any limit at all. The 255 limit of the paper is based on constraints that are not really relevant in 2024 anymore.
| val joinsOf = | ||
| new mutable.LinkedHashMap[IRNames.ClassName, mutable.LinkedHashSet[IRNames.ClassName]]() |
There was a problem hiding this comment.
AFAICT, the algorithm does not rely on a particular iteration order for joinsOf or its contained sets (even for stability). Therefore, mutable.HashMap and mutable.HashSet (instead of their Linked variants) would be better, since they are more efficient.
| val usedOf = new mutable.LinkedHashMap[IRNames.ClassName, mutable.LinkedHashSet[Bucket]]() | ||
| val spines = new mutable.LinkedHashSet[IRNames.ClassName]() |
There was a problem hiding this comment.
Same here. AFAICT HashMaps and HashSets are enough.
| val className = clazz.name.name | ||
| val info = getClassInfo(className) | ||
| (if (clazz.kind == ClassKind.Interface) List(className) else Nil) ++ | ||
| info.ancestors.collect { |
| def getAllInterfaces(clazz: LinkedClass): List[IRNames.ClassName] = { | ||
| val className = clazz.name.name | ||
| val info = getClassInfo(className) | ||
| (if (clazz.kind == ClassKind.Interface) List(className) else Nil) ++ |
There was a problem hiding this comment.
Why is this necessary? I'm pretty sure ancestors always contains the class/interface itself. Isn't that the case?
There was a problem hiding this comment.
Oh, I misunderstood something, indeed we don't need this
| while (!found && bs.hasNext) { | ||
| val b = bs.next() | ||
| // two spine types can share a bucket only if they don't have any common join type descendants | ||
| if (b.size < Bucket.MAX_SIZE && b.joins.intersect(joins).isEmpty) { |
There was a problem hiding this comment.
| if (b.size < Bucket.MAX_SIZE && b.joins.intersect(joins).isEmpty) { | |
| if (b.size < Bucket.MAX_SIZE && !b.joins.exists(joins)) { |
No need to actually construct the intersection set.
| def assignBuckets(classes: List[LinkedClass]): Unit = | ||
| _buckets = assignBuckets0(classes) |
There was a problem hiding this comment.
Perhaps immediately filter out all classes that are .kind.isJSType. It is a bit hard to reason about what the algorithm will do with JS types, as is, but in fact it shouldn't do anything at all.
This commit implements the "packed encoding" algorithm for compressing interface tables (itables). The algorithm is based on the paper "Efficient Type Inclusion Tests". https://www.researchgate.net/publication/2438441_Efficient_Type_Inclusion_Tests It compresses the itable by reusing itable indices for unrelated types, reducing the size of the itable array. For scalajs-test-suite, the itable size is reduced from 413 to 47. This change didn't reduce the size of wasm binary that much (14957kb to 14943kb), but it should be space efficient. We chose packed encoding because the compression rate is good enough among the presented methods in the paper. While hierarchical encoding is slightly better compression rate, packed encoding is better in both compile-time and runtime performance in CPU and space.
tanishiking
force-pushed
the
compress-itables
branch
from
10113cd to
3bd0321
Compare
This commit implements the "packed encoding" algorithm for compressing interface tables (itables). The algorithm is based on the paper "Efficient Type Inclusion Tests". https://www.researchgate.net/publication/2438441_Efficient_Type_Inclusion_Tests It compresses the itable by reusing itable indices for unrelated types, reducing the size of the itable array. For scalajs-test-suite, the itable size is reduced from 413 to 47.
This change didn't reduce the size of wasm binary that much (14957kb to 14943kb), but it should be space efficient.
We chose packed encoding because the compression rate is good enough among the presented methods in the paper. While hierarchical encoding is slightly better compression rate, packed encoding is better in both compile-time and runtime performance in CPU and space.