Closed Bug 887533 Opened 7 years ago Closed 7 years ago
Include even less stuff in dom/bindings
This only implements #1 from https://bugzilla.mozilla.org/show_bug.cgi?id=876555#c8 but it appears to be enough.
Before Kyle Huey@KYLEHUEY-PC obj-i686-pc-mingw32/dom/bindings/.deps $ cat *.obj.pp | wc -l 469262 After Kyle Huey@KYLEHUEY-PC obj-i686-pc-mingw32/dom/bindings/.deps $ cat *.obj.pp | wc -l 327406
I'm a little surprised that's enough. Do we just not hit #2 and#3, or do they compile for some reason?
I think we just don't hit it. The only thing with hasXPConnectImpls today is EventTarget.
Ah, because we fixed Event, right. And we have fewer and fewer external interfaces, I guess... Alright, then!
Comment on attachment 768046 [details] [diff] [review] Patch I'm willing to just ignore the edge cases if you are ...
Attachment #768046 - Flags: review?(bzbarsky)
Comment on attachment 768046 [details] [diff] [review] Patch Hmm. So those #includes are needed because those types are the return values of GetParentObject? I guess I don't have a better proposal for those, short of reducing the number of types we ever use for parent objects (basically to nsINode and nsPIDOMWindow or something) and including them in all all the binding cpp files. But I guess that for something like nsDOMAttributeMap even if we wanted to return nsINode* we'd need to include Element.h so that we know Element inherits from nsINode. :( Another option is to out-of-line GetParentObject methods as needed, make them return ParentObject, and then the implementation is the only thing that needs to know about the actual types involved. This does mean an extra out-of-line call when wrapping, though. r=me either way.
Attachment #768046 - Flags: review?(bzbarsky) → review+
Without this patch, I get build times like so for dom/bindings: 1117.313u 100.303s 5:08.81 394.2% 0+0k 115+8662io 0pf+0w 1121.414u 100.267s 5:09.96 394.1% 0+0k 352+7266io 0pf+0w With the patch I get build times like so: 661.799u 63.282s 3:05.31 391.2% 0+0k 1241+5981io 390pf+0w 644.380u 63.480s 3:00.86 391.3% 0+0k 442+6365io 0pf+0w I filed bug 901299 on maybe fixing up the GetParentObject pain points.
Attachment #785473 - Attachment is obsolete: true
Are there any more low-hanging #include fruit here? ;)
Comment on attachment 794708 [details] [diff] [review] This should fix the test orange Review of attachment 794708 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bindings/Codegen.py @@ +8639,5 @@ > + return > + needsDOMQS["value"] = typeDesc.hasXPConnectImpls > + > + callForEachType(descriptors + callbackDescriptors, dictionaries, > + workerCallbacks + mainCallbacks, Why do we need workerCallbacks here if only mainthread things can have hasXPConnectImpls?
Attachment #794708 - Flags: review?(khuey) → review+
> Why do we need workerCallbacks here We don't. Removed. https://hg.mozilla.org/integration/mozilla-inbound/rev/15380522db96
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Fwiw, this push reduced the memory needed to link libxul by about 120MB (!).
Not sure whether to be happy or befuddled.
Happy. ;) Other include-reductions are also producing drops in the libxul link memory, just smaller ones.
(In reply to comment #14) > Fwiw, this push reduced the memory needed to link libxul by about 120MB (!). Yeah, it looks like the PGO compiler in the first phase just dumps out the entire AST into the object files, or something crazy like that!
You need to log in before you can comment on or make changes to this bug.