CSS2PropertiesBinding by itself is massive, some 600K of stuff on x86-64 Linux (readelf -sW | grep CSS2PropertiesBinding). One of the reasons for that is things like: static bool get_webkitUserSelect(JSContext* cx, JS::Handle<JSObject*> obj, nsDOMCSSDeclaration* self, JSJitGetterCallArgs args) { binding_detail::FastErrorResult rv; DOMString result; self->GetWebkitUserSelect(result, rv); ... static bool get_WebkitUserSelect(JSContext* cx, JS::Handle<JSObject*> obj, nsDOMCSSDeclaration* self, JSJitGetterCallArgs args) { binding_detail::FastErrorResult rv; DOMString result; self->GetWebkitUserSelect(result, rv); ... static bool get__webkit_user_select(JSContext* cx, JS::Handle<JSObject*> obj, nsDOMCSSDeclaration* self, JSJitGetterCallArgs args) { binding_detail::FastErrorResult rv; DOMString result; self->GetWebkitUserSelect(result, rv); ... That is, for three different properties (-webkit-user-select, WebkitUserSelect, webkitUserSelect) that all have exactly the same underlying implementation, we've codegenned three different functions, three different sets of jitinfo, etc. I don't believe these identical functions can be folded together by the linker with our current settings, since their addresses are taken by (generated) JS engine structures. We shouldn't really need to do this; we should be able to generate one canonical version and then direct the distinct JS properties to said version. Eyeballing the generated CSS2Properties.webidl suggests that we could trim it down to ~half or less of its current size. Sadly, this doesn't apply to virtually any other places in bindings, but trimming down CSS2PropertiesBinding on its own would be nothing to sneeze at.

Looks like none of these things use the actual property name, so we could in fact do this. Nathan, do you want to give me a shot, or should I do it?

I looked into this a bit out of curiosity. > I don't believe these identical functions can be folded together by the linker with our current settings, since their addresses are taken by (generated) JS engine structures. While this is true, the compiler (or at least my GCC5) is able to deduplicate the functions - the duplicates simply JMP to the actual implementation. Because of this, the code duplication doesn't impact binary size too much. I still implemented it as a PoC/hack, to measure the improvement - I made WebIDL support Alias= for attributes and used it to generate a smaller.webidl: > [... Alias="WebkitUserSelect" Alias="-webkit-user-select"] attribute DOMString webkitUserSelect; The generated .webidl and .cpp half were half of their current size. The binary size reduction was: Reported by readelf (readelf -sW | grep CSS2PropertiesBinding): 38kB (450kB -> 412kB) Size of UnifiedBindings1.o: 75kB (898kB -> 823kB) If you think it's still worth the effort and that my approach was good, I could upload the PoC patch.

75kB is a very nice codesize win, if it's still that big by the time we're done linking libxul. If it carries over to that, it would be worth doing. One caveat is that we'd lose the ability to separately use-count aliased API calls, but we could just unalias things we need to use-count while we're collecting data.

On GCC5 x64, after `mach build` and stripping libxul.so, I'm getting a 106kB filesize difference (130.515 MB -> 130.409 MB).

Thanks for looking at this! This is very cool that GCC5 deduplicates code at the compiler level. Even with that, the results from comment 4 are very encouraging! I'd suggest two refinements, one of which may already be featured in the measurement: 1) Ensure that --enable-release is in your mozconfig, so various optimizations that are present in our release builds of Firefox are present in your local build; it wasn't explicitly stated whether this option was active. (I'm assuming you're already compiling with --enable-optimize --disable-debug.) 2) Measuring total size of the binary is ok; I find it easier to measure sizes with `size`, which will report text (code and read-only data)/data (writable data)/bss (zero-initialized writable data) sizes and doesn't require the binary to be stripped beforehand. Can you please ensure that your builds are using #1 and report back the numbers you get with #2? Thanks!

Looks good, thank you! Please post the patch and ask :bz for review.

Comment on attachment 8943748 [details] Bug 1297480 - Add Alias= for WebIDL attrbutes, use it to slim CSS2Properties down. Needs feedback on general approach. Todo: - changes to length() are very hacky; probably needs some bigger refactor (suggestions?) - split into two commits (one for WebIDL, one for GenerateCSS2Properties) - update comments

Comment on attachment 8943748 [details] Bug 1297480 - Add Alias= for WebIDL attrbutes, use it to slim CSS2Properties down. https://reviewboard.mozilla.org/r/214132/#review220088 This generally seems reasonable. I'm not too enthused about having Alias on both methods (operations) and attributes with subtly different semantics: on methods it causes both names to point to the same JS Function, while the proposal for attributes here has the getter/setters for the different attributes distinct. So it would be best to come up with a different name for the extended attribute. We should also do some validation similar to what we already do for the method case in IDLInterfaceOrNamespace.validate. ::: dom/bindings/Codegen.py:2160 (Diff revision 1) > > def length(self, chrome): > - return len(self.chrome) if chrome else len(self.regular) > + arr = self.chrome if chrome else self.regular > + length = len(arr) > + for val in arr: > + if hasattr(val, 'aliases'): Won't this also increase the length for methods that have aliases? We didn't use to do that.... why do we want to start? ::: dom/bindings/GenerateCSS2PropertiesWebIDL.py:58 (Diff revision 1) > # > # Note that "float" will cause a property called "float" to exist due to (1) > # in that list. > # > # In practice, cssFloat is the only case in which "name" doesn't contain > # "-" but also doesn't match "prop". So the above generatePropLine() call The generatePropLine call is no longer "above". Please adjust the comment?

Comment on attachment 8943748 [details] Bug 1297480 - Add Alias= for WebIDL attrbutes, use it to slim CSS2Properties down. This generally seems like a great approach!

Comment on attachment 8944852 [details] Bug 1297480 - Add BindingAlias= for WebIDL attrbutes, use it to slim CSS2Properties down. https://reviewboard.mozilla.org/r/214998/#review221830 r=me with the nits below fixed. This is great; I really like the resulting codegen. For anyone else following along: { "webkitTextFillColor", JSPROP_ENUMERATE, GenericBindingGetter, &webkitTextFillColor_getterinfo, GenericBindingSetter, &webkitTextFillColor_setterinfo }, { "WebkitTextFillColor", JSPROP_ENUMERATE, GenericBindingGetter, &webkitTextFillColor_getterinfo, GenericBindingSetter, &webkitTextFillColor_setterinfo }, { "-webkit-text-fill-color", JSPROP_ENUMERATE, GenericBindingGetter, &webkitTextFillColor_getterinfo, GenericBindingSetter, &webkitTextFillColor_setterinfo }, ::: commit-message-94057:2 (Diff revision 1) > +Bug 1297480 - Add BindingAlias= for WebIDL attrbutes, use it to slim CSS2Properties down. r?bz > + Would be good to write a description of what the effect of BindingAlias is in observable terms in the commit message. ::: dom/bindings/Codegen.py:2735 (Diff revision 1) > def generateArray(self, array, name): > if len(array) == 0: > return "" > > + def condition(m, d): > + return PropertyDefiner.getControllingCondition(m["attr"], d) OK. So this is going to mean that we can't do different conditional exposure on a thing and its BindingAlias. That's probably OK for now. ::: dom/bindings/GenerateCSS2PropertiesWebIDL.py:58 (Diff revision 1) > # > # Note that "float" will cause a property called "float" to exist due to (1) > # in that list. > # > # In practice, cssFloat is the only case in which "name" doesn't contain > # "-" but also doesn't match "prop". So the above generatePropLine() call There is no generatePropLine call above anymore. Please fix these comments to describe the new code.

And my apologies for the lag here....

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7bc8e64beb7da58783defa2f2b774156f988848e

Thank you for the update! Autolanding requested on the patch. And thank you again for doing this.

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/187eb3dff851 Add BindingAlias= for WebIDL attrbutes, use it to slim CSS2Properties down. r=bz

Documented at https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#BindingAlias