Help needed / Less set garbage

[mbouaziz]

I was trying to reduce the temporary garbage added by #886 by optimizing SortedSet/Map when the same element is added over and over again, by using physical equality as I would do in OCaml, though SKDB wasm tests are not happy with it, they throw an unreachable exception, e.g.

 Error: page.evaluate: unreachable executed
    .LSKIP_physEq_bitcast_invalid@http://127.0.0.1:8100/node_modules/skdb/dist/skipwasm-std/sk_types.js line 550 > WebAssembly.instantiate:wasm-function[1681]:0x20afb
    sk.SortedMap_Node__setWith.26@http://127.0.0.1:8100/node_modules/skdb/dist/skipwasm-std/sk_types.js line 550 > WebAssembly.instantiate:wasm-function[2466]:0x39616

Anyone knowledgeable with compilation to WASM has an idea on this?

[mbouaziz]

Thanks @beauby for helping me on this!
The conclusion is: it doesn't work with value classes.

For example, calls to phys_eq for a pair is transformed to a call with 4 parameters instead of 2.

E.g. in getLatestDescr, the line !tableMap.map[baseName] = (tableMap.version, result); calls SortedMap#set with the value type V = (Int, Descr).
The generated LLVM is

define i8* @sk.SortedMap_Node__setWith.20(i8* %this.0, i8* %key.1, i64 %value.i0.value.2, i8* %value.i1.3, i8* %f.4) unnamed_addr uwtable personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) !dbg !19758 {

, we can see

%r38 = tail call zeroext i1 @SKIP_physEq(i64 %value.i0.value.2, i8* %value.i1.3, i64 %r14, i8* %r15), !dbg !19769

[mbouaziz]

A solution would be to box stuff before passing it to phys_eq which would actually be a shallow_eq, though this would create garbage and the goal of checking physical equality was to reduce garbage

[jberdine]

Is this the problematic call, or are there others? This one could perhaps be replaced with the == from the Equality trait on V without being slow.

[mbouaziz]

All the calls to phys_eq are problematic for value class objects (as explained in #900).

I tried to check contains before adding elements but it didn't change the memory footprint, so there must be another issue in #886.

[jberdine]

Ok, I guess the thrust of my question is: is this the only call to phys_eq added in this PR for a value class?

[mbouaziz]

Yes it is.

By the way, there is currently no constraint that V implements Equality