Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-consider the indexing of local variables, giving each type its own dense index space. #509

Closed
ghost opened this issue Dec 23, 2015 · 7 comments

Comments

@ghost
Copy link

ghost commented Dec 23, 2015

The type of the get_local operation seems easy to predict from its context. Even if not certain, a compressor can make a useful prediction. Also a compressor is using the prior context, not the arguments which are yet to be encoded. Thus the compressor is effectively working with sparse sets of index spaces which might be frustrating it.

Most of the operators have a unique symbol for each result type defined by the parent node, yet get_local seems to have the type defined by the local variable index. I can see how this helped with the particular encoding used in the polyfill-prototype-1 which had a one byte immediate encoding for just four opcodes and effectively used the index to give the type information and avoided having i32.get_local etc. But perhaps in the context of a better compressors this might frustrate efficient encoding.

Even the polyfill-prototype-1 could have done better here by looking at the context and only when the type was certain from the context it could have encoded using the immediate opcodes and dense index spaces, and this would have handled the high frequency cases more efficiently. It would have fallen back to a longer encoding when the type was not certain, which seems to be low frequency.

@lukewagner
Copy link
Member

I can see how this helped with the particular encoding used in the polyfill-prototype-1

polyfill-prototype-1 actually does the opposite of what you seem to think: there is a separate opcode space for each type and the parent's type context determines which opcode space to index. This is quite different than what's in AstSemantics.md and in the binary format proposed in #497, but, as I've mentioned in several other comments on several other issues, I'm not longer in favor of this strategy. In short: I think that with module-local opcode tables, there won't be any size advantage. Anyhow, it's something that bears further measurement.

@ghost
Copy link
Author

ghost commented Dec 23, 2015

@lukewagner polyfill-prototype-1 does not specialize the get_local operation based on the parent type, but could have and then could have just as efficiently supported i32.get_local, f64.get_local, and f32.get_local each with separate index spaces and thus made better use of the immediate one byte encoding.

I am a bit sceptical now that a user defined decompressor will be practical for most wasm deployments in the medium term. The polyfill-prototype-1 file size is almost half that of #497 before compression. Is there a working assumption that this natural encoded size is not of great importance and that the natural encoding be optimized for the standard http compression methods such as gzip and perhaps brotli?

Knowing the target compressors might help designing the pre-compressed natural format. For example, strategies such as placing all the function local variable index references together in a block for each function, etc.

@lukewagner
Copy link
Member

@lukewagner polyfill-prototype-1 does not specialize the get_local operation based on the parent type

Yes, it does; expr_i32 is called when the parent implies the operands are children.

@ghost
Copy link
Author

ghost commented Dec 24, 2015

@lukewagner Yes, but the point is that all of the i32, f64 and f32 paths dispatch to the same get_local operation - it could differentiate the index spaces but doesn't. See:

expr_i32(State& s, Prec prec, Ctx ctx) { ... case I32::GetLoc: get_local(s); break;
expr_f32(State& s, Prec prec, Ctx ctx) { ... case F32::GetLoc: get_local(s); break;
expr_f64(State& s, Prec prec, Ctx ctx) { ... case F64::GetLoc: get_local(s); break;

@lukewagner
Copy link
Member

Sure, but the type isn't defined by the local index; e.g., it would be invalid to have I32::GetLoc index a non-I32 and thus easy to have overlapping index spaces for each local type. Anyhow, my point with all this isn't to discuss polyfill-prototype-1 but to point out that the current design isn't, in fact, motivated by constraints of polyfill-prototype-1.

Rather, the motivations have already been discussed at some length in #408 and #182.

@ghost
Copy link
Author

ghost commented Dec 24, 2015

@lukewagner Ok, I think I understand your point now, that polyfill-prototype-1 is not using the index to determine the type rather it knows with certainty the type from the context. Thank you for your patience.

Perhaps the second issue you referred to should have been #311 which seemed to discuss the same issue as here, but seemed much more speculatively, and did not take into account the compression and encoding efficiency.

@sunfishcode
Copy link
Member

This topic is being tracked in #309.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants