904962 - Avoid some "critical" #includes in and around SpiderMonkey

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Attachment 789006 [details] from bug 901132 has a list of #include statements listed by "criticality".  For each #include statement, if it were removed, the number indicates (approximately) how many fewer times we would read the corresponding header during a browser build.

I have some patches that remove some of the critical #includes, which are:

- 2891 js/public/CharacterEncoding.h from js/src/jsapi.h 
- 2136 mfbt/ThreadLocal.h from js/src/jsapi.h 
- 173 js/src/yarr/YarrSyntaxChecker.h from js/src/vm/RegExpObject.h 
- 148 --GENERATED--/js/src/jsautooplen.h from js/src/vm/Stack.h
-  73 js/src/vm/String-inl.h from js/src/gc/Barrier-inl.h

Plus a couple of other patches that remove unnecessary #includes that I had lying around.

With these, I see a 1.5% speed-up when building SpiderMonkey.  But there will also be effects outside SpiderMonkey by removing the first two cases in the list above.

I have a similar analysis script that works just within SpiderMonkey.  Here are some notable changes in how often some headers are included during a JS shell build, once these patches are applied:

vm/MatchPairs.h:          176 --> 7
vm/String-inl.h:          112 --> 10
yarr/YarrParser.h:        178 --> 3
yarr/YarrSyntaxChecker.h: 177 --> 2

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Don't #include js/CharacterEncoding.h in jsapi.h

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Don't #include mozilla/ThreadLocal.h in jsapi.h.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Minimize #includes in vm/RegExpObject.h.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Don't #include jsautooplen.h in vm/Stack.h.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 5) - Don't #include vm/String-inl.h in gc/Barrier-inl.h.

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: (part 5) - Don't #include jsprf.h in three places it isn't needed.

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 789951 [details] [diff] [review]
(part 4) - Don't #include jsautooplen.h in vm/Stack.h.

Review of attachment 789951 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/Stack.cpp
@@ +450,5 @@
>  }
>  
>  /*****************************************************************************/
>  
> +// Unlike the other methods of this calss, this method is defined here so that

class

Comment 8

[Nicholas Nethercote [inactive]]

Comment on attachment 789952 [details] [diff] [review]
(part 5) - Don't #include vm/String-inl.h in gc/Barrier-inl.h.

> (part 5) - Don't #include vm/String-inl.h in gc/Barrier-inl.h.

Try server tells me that this patch breaks the world, so I won't include it when I land the others.

Comment 9

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c084ee924106
https://hg.mozilla.org/integration/mozilla-inbound/rev/71aaddc68ce0
https://hg.mozilla.org/integration/mozilla-inbound/rev/621142a4590c
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ea28db5b5cb
https://hg.mozilla.org/integration/mozilla-inbound/rev/82a32598ad71

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c084ee924106
https://hg.mozilla.org/mozilla-central/rev/71aaddc68ce0
https://hg.mozilla.org/mozilla-central/rev/621142a4590c
https://hg.mozilla.org/mozilla-central/rev/0ea28db5b5cb
https://hg.mozilla.org/mozilla-central/rev/82a32598ad71