Closed Bug 908050 (minimize-jsapi) Opened 11 years ago Closed 11 years ago

Minimize number of files that depend on jsapi.h

Categories

(Core :: General, defect)

Product:
Core
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla27

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

(Blocks 1 open bug)

Details

(Whiteboard: [js:t])

I started minimizing the number of files that depend on jsapi.h in bug 905017. But it's a big task, so here's a tracking bug.
Depends on: 903283
I'm going to track two metrics as I land patches. - "rebuilt" is the number of files that get rebuilt after touching jsapi.h. This is the real metric of interest. - "includes" is the number of times jsapi.h gets included during a build, assuming no only-load-a-header-once optimization. (I determine this by adding a |#warning| statement to the top of jsapi.h before building, which defeats said optimization.) Because jsapi.h can be transitively included many times per .cpp file, often a single patch will affect "includes" significantly without affecting "rebuilt" significantly. The changes due to bug 905017 are as follows. landed as rebuilt includes starting point 143783:493cd772def7 2631 89802 905017-part-1 143784:dba0f0715b18 2522 40146 ... 905017-part-2 143793:7e6fb33fdf22 2500 38291 905017-part-3 143794:c85332df4320 2222 37471
Depends on: 909090
Alias: minimize-jsapi
Depends on: 909178
Depends on: 909597
Depends on: 909623
After bug 909178: > landed as rebuilt includes > starting point 143783:493cd772def7 2631 89802 > 905017-part-1 143784:dba0f0715b18 2522 40146 > ... > 905017-part-2 143793:7e6fb33fdf22 2500 38291 > 905017-part-3 143794:c85332df4320 2222 37471 > ... > 909178-part-1 144434:7f8e99aec954 1563 29371 > 909178-part-2 144435:f8fec7c369d1 1563 29383 Looks like the work in other bugs is having an effect. Cool.
Depends on: 910109
After bug 909597: > landed as rebuilt includes > starting point 143783:493cd772def7 2631 89802 > 905017-part-1 143784:dba0f0715b18 2522 40146 > ... > 905017-part-2 143793:7e6fb33fdf22 2500 38291 > 905017-part-3 143794:c85332df4320 2222 37471 > ... > 909178-part-1 144434:7f8e99aec954 1563 29371 > 909178-part-2 144435:f8fec7c369d1 1563 29383 > ... > (predecessor) 144620:3e064e6246aa 2157 34618 > ... > 909597-part-2 144622:2a3ed74a0c95 2157 33801 So bug 909597's effect was minimal. I don't know what happened between r144435 and 1444620 :(
Depends on: 908576
I did some bisecting... > > 905017-part-3 143794:c85332df4320 2222 37471 > > ... > > 909178-part-1 144434:7f8e99aec954 1563 29371 > > Looks like the work in other bugs is having an effect. Cool. That was due to patch 7 in bug 908576, courtesy of bz. > > 909178-part-2 144435:f8fec7c369d1 1563 29383 > > ... > > (predecessor) 144620:3e064e6246aa 2157 34618 > > I don't know what happened between r144435 and 1444620 :( That was due to bug 909514, the patch for which accidentally adding |#include "jsapi.h"| to BindingDeclarations.h. I landed a follow-up patch that removed it again.
Current status: > landed as rebuilt includes > (predecessor) 143783:493cd772def7 2631 89802 > 905017-part-1 143784:dba0f0715b18 2522 40146 minimize js header inclusions > ... > 905017-part-2 143793:7e6fb33fdf22 2500 38291 create js/StructuredClone.h > 905017-part-3 143794:c85332df4320 2222 37471 create js/ProfilingStack.h > > (predecessor) 2223 34759 > 908576-part-7 144195:08e3075e7320 1558 29303 move DOMProxyHandler classes > ... > 909178-part-1 144434:7f8e99aec954 1563 29371 create js/Id.h > 909178-part-2 144435:f8fec7c369d1 1563 29383 do-jsclass > ... > (predecessor) 144523 1563 29376 > 909514 144524 2156 34589 add jsapi to BindingDeclarations.h > > (predecessor) 144620:3e064e6246aa 2157 34618 > ... > 909597-part-2 144622:2a3ed74a0c95 2157 33801 do-jsdbgapi > > (predecessor) 144867:9b1af0e6afd4 2146 29563 > 910109-part-1 144868:73bad4a03382 2146 31302 do-jsproxy > 910109-part-2 144869:d7f48284d66a 2146 28937 do-jswrapper > 909514-followup 144870:fc2901f48146 1539 23778 rm jsapi from BindingDeclarations.h
Depends on: 909514
jcranmer updated the data in bug 901132... between August 4th and August 30th, the number of static transitive includes of jsapi.h dropped from 3434 to 1792.
Depends on: 912411
No longer depends on: 909090
After bug 912411: > landed as rebuilt includes > (predecessor) 143783:493cd772def7 2631 89802 > 905017-part-1 143784:dba0f0715b18 2522 40146 minimize js header inclusions > ... > 905017-part-2 143793:7e6fb33fdf22 2500 38291 create js/StructuredClone.h > 905017-part-3 143794:c85332df4320 2222 37471 create js/ProfilingStack.h > ... > (predecessor) 2223 34759 > 908576-part-7 144195:08e3075e7320 1558 29303 move DOMProxyHandler classes > ... > 909178-part-1 144434:7f8e99aec954 1563 29371 create js/Id.h > 909178-part-2 144435:f8fec7c369d1 1563 29383 do-jsclass > ... > (predecessor) 144523 1563 29376 > 909514 144524 2156 34589 add jsapi to BindingDeclarations.h > ... > (predecessor) 144620:3e064e6246aa 2157 34618 > ... > 909597-part-2 144622:2a3ed74a0c95 2157 33801 do-jsdbgapi > ... > (predecessor) 144867:9b1af0e6afd4 2146 29563 > 910109-part-1 144868:73bad4a03382 2146 31302 do-jsproxy > 910109-part-2 144869:d7f48284d66a 2146 28937 do-jswrapper > 909514-followup 144870:fc2901f48146 1539 23778 rm jsapi from BindingDeclarations.h > ... > (predecessor) 145772:55cdc3eef815 1533 20314 > 912411-part-1 145773:5414483004cf 1487 19350 rm some jsapis from Gecko > 912411-part-2 145774:053f6a2c5e29 1487 19158 rm jsapi from AccessCheck > 912411-part-3 145775:c1262e2db291 1484 15396 rm some jsapis within SM
Depends on: 918330
After bug 918330: > ... > (njn moved to a new machine) > ... > (predecessor> 147955:11feb444914d 1510 16487 > 918330 147956:08d8559d2493 1294 15739 GfxInfoCollector.h
Currently we have just under 1300 files rebuilt when jsapi.h is touched. To get that down by more than a handful, we need to make the following files not depend on jsapi.h. - nsCxPusher.h: Uses JSAutoRequest and JSAutoCompartment, which can be moved from jsapi.h into new headers. - xpcpublic.h: Uses JS_NewExternalString() and JS_GetEmptyStringValue(). Probably not hard to remedy, though I haven't done it. - DOMJSProxyHandler.h: uses JSPropertyDescriptor, which can be moved from jsapi.h into a new header. - Workers.h: uses JSGCParamKey, which can be moved from jsapi.h into a new header. - BindingUtils.h: This is the hard one -- it uses JSObjectIsDate(), JS_ObjectIsRegExp(), JS_WrapValue(), JS_IsExceptionPending(), JS_ValueToString(), JS_GetStringCharsAndLength(), JS_InternString(), INTERNED_STRING_TO_JSID(), JS_GetSringCharsZAndLength(), JSID_TO_FLAT_STRING(), JS_FlatStringEqualsAscii(), JS::AutoIdVector, JS_IsNativeFunction(), and CustomAutoRooter. - CallbackFunction.h: JS_ObjectIsCallable() (in assertion) -- easy. This drops the number of rebuilt files from 1269 to 1080. - TypedArray.h: Uses CustomAutoRooter (like BindingUtils.h). This drops the rebuild count to 977. And at this point, the majority of jsapi.h includers at build-time 525 are .cpp files, with another 200-odd in SpiderMonkey itself.
In other words, if we perform some heroics, we can get it down from just under 1300 to just under 1000. Probably not worth it. I'm tempted to mark this as fixed -- it was over 2600 when this bug was filed.
Yeah, I agree, there is probably not a lot of low hanging fruit to be picked here any more...
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Part 3 of bug 907789 reduced the number of files rebuilt after touching jsapi.h from 1299 to 890. AIUI, that's because lots of the dom bindings files are now combined, and so although the number of files is down, the average size of those files is up, and in the end it's not much different in terms of compile time. Is that right?
(In reply to Nicholas Nethercote [:njn] from comment #12) > Part 3 of bug 907789 reduced the number of files rebuilt after touching > jsapi.h from 1299 to 890. AIUI, that's because lots of the dom bindings > files are now combined, and so although the number of files is down, the > average size of those files is up, and in the end it's not much different in > terms of compile time. Is that right? The number of files is down, the average size is up, but it was a significant win in total compile time when I measured it locally. I think bz has also mentioned how much more pleasant full rebuilds of dom/bindings are with bug 907789. But I am willing to believe the possibility of it being slower.
> The number of files is down, the average size is up, but it was a > significant win in total compile time when I measured it locally. I just measured and you're right. Before, rebuilding after touching jsapi.h took 5m58s; after, it took 4m53s. Cool!
You need to log in before you can comment on or make changes to this bug.