More webgl code size opts

[juj]

Should not be squashed when merging to help bisection if I made an oopsie anywhere. Saves ~3% of minimal WebGL demo code size.

[kripken]

Nice work, everything I didn't comment on looks good.

[kripken]

Nice to rely on textures[0] === null!

Please add a comment in the source where we define those arrays, that index 0 contains null for that purpose (reduce the risk of later confusion).

[juj]

There is already a comment counter: 1, // 0 is reserved as 'null' in gl that has been around for a long time it looks like.

[kripken]

why is this change shorter? just because Math.max does not minify down in closure?

if that's the issue, I'd rather not do this, as it decreases readability. Instead closure, or we, could do var x = Math.max once at the toplevel, and we'd use the minified name here.

[juj]

That's right, Math.max does not minify since it is an extern. Without Math.max the code becomes

var h=L.getActiveUniform(b,d),k=h.name,l=k.length+1;l>a.L&&(a.L=l);l=k.lastIndexOf("[")

whereas with Math.max() code is

var h=L.getActiveUniform(b,d),k=h.name;a.L=Math.max(a.L,k.length+1);var l=k.lastIndexOf("[");

Closure has a bug there it looks like, it could have done

var h=L.getActiveUniform(b,d),k=h.name,l;a.L=Math.max(a.L,k.length+1);l=k.lastIndexOf("[");

to save 2 more bytes, but that would still be 4 bytes longer than the version without Math.max.

Also,

var x=Math.max;p.m=x(p.m,l);

would be longer than just

p.m=Math.max(p.m,l);

so the assignment var x=Math.max; is only shorter if it can be amortized between multiple uses of Math.max. Searching candidates to amortize with shows only emscripten_set_timeout_loop() in asm.js mode as worthwhile, but not many apps would necessarily use that.

Code pattern

var length = ...;
if (length > max) {
  max = length;
}

does not look too unusual, it is after all what every beginner programmer writes when they start out and are not yet familiar with the language's built-in math primitives. Closure is just telling they are not wrong :)

[kripken]

What about Math_max, though? with a _ there, closure should minify it properly? We do already define that, so you can just use it.

[juj]

var Math_max=Math.max; will minify down to var x=Math.max; from above. The define does not come free, it is not getting used anywhere, and it's getting optimized out. If it was not optimized out, it still would need to amortize for it to warrant its weight.

[kripken]

It's true we'd still have var x=Math.max, but that's a single cost compared to if(a<b)a=b; in multiple places (which we'd replace with a=x(a,b);. So with enough uses that's better. I don't think we should bet against not having more uses.

If we do want to super-optimize stuff like this, see my other comment about the new estree optimizer - we could write a specific pass to optimize this exact thing, to see how many max operations there are, and pick between the two forms globally.

[juj]

Hmm, I think that would be a complete overkill in terms of complexity.. It should be much simpler to have a if (x > max) max = x; in code, compared to writing a full optimization pass to do the job?

[kripken]

I think this would be clearer with Math.infinity, and we can assume closure will minify it (if not, we can file a bug, and could temporarily force it to do so with a toplevel var Math_infinity = Math.infinity and use Math_infinity here.

[juj]

It would be clearer with infinity, but similarly outlining var Math_infinity = Infinity; would be shorter only if multiple uses of Math_infinity could amortize the hit. (also all uses of such var Math_infinity should likely have to be in cold program paths, because a JIT can see that Infinity is always a constant literal, whereas a dereference via a Math_infinity could sneakily change under it, so it could not do JIT magic on such a variable)

9e9 is pretty large, no? :)

[kripken]

It seems safer to add Math_infinity and use that - otherwise 9e9 will eventually get us in trouble, or at least be confusing to read - it just looks arbitrary.

Another option may be to do var VERY_LARGE = 9e9; with a big comment on that, and use that in places like this. Or is this really the only one?

[juj]

That code site has several 'i32 accesses right next to it, so I am not concerned e.g. about wasm64 or similar. This is currently the only place in code that has a construct like this.

var VERY_LARGE = 9e9; is a good idea, but we cannot do it until google/closure-compiler#3187 is resolved. (Having a function local define and use of VERY_LARGE is practically about the same as 9e9/*VERY_LARGE*/)

[kripken]

If we can't rely on closure for such an optimization, another option is to write one ourselves. #7973 has a new estree-based optimization infrastructure, which is much nicer to write passes in (has a proper AST, instead of uglify1's node[1][3][2] etc.).

[juj]

We cannot write a generic optimizer pass to optimize Infinity to 9e9, because we don't know about the use case, In this specific location 9e9 is large enough to serve as infinity, but in other places it may not be?

[juj]

Oh you meant specifically for the literal expansion of a VERY_LARGE variable.. I am not sure that it would be something I can jump on right now, and the worth is not that high, especially since it would be to automatically optimize what was trivial to write in code in the first place.

[juj]

Updated the PR, what are your thoughts on a 2nd pass?

[juj]

I think it is probably simplest and fastest to avoid the questionable optimizations. Pushed a revert to those, it would be good to land this to be able to bring the different optimizations together in the tree.

[kripken]

lgtm. I think we should think more about how to optimize stuff like max etc. - we can do more stuff than closure can, because it is fully general, while we could make stronger assumptions in our code (like Math.min not being changed).

[juj]

Yeah, that sounds like a good path forward.