Re: Stop #including jsapi.h everywhere!
On 2013-08-19 9:10 PM, Gregory Szorc wrote:
On 8/19/13 5:15 PM, Nicholas Nethercote wrote:
Hi,
Analysis in https://bugzilla.mozilla.org/show_bug.cgi?id=901132 has
indicated that jsapi.h is probably responsible for more recompilation
than any other file in the Mozilla tree -- it gets included in *lots*
of files, and it gets modified very often, partly because it's so
large. (jsfriendapi.h is also bad in this respect.)
I'm trying to improve this situation in
https://bugzilla.mozilla.org/show_bug.cgi?id=905017. It turns out
that *many* |#include jsapi.h| statements are simply not needed.
They can be replaced by forward declarations of a handful of types,
e.g.:
struct jsid;
struct JSContext;
class JSObject;
namespace JS {
template typename T class Handle;
template typename T class MutableHandle;
class Value; // #include js/Value.h (not jsapi.h) if you need the
*definition*
}
Next time you're thinking of adding a |#include jsapi.h| statement,
please think about whether a forward declaration would suffice -- i.e.
if you are only using public JS types (i.e. not functions), and only
using them as pointers, references, or parameters in function
declarations. (Forward-declaring JS_PUBLIC_API types is harder; ask
me for help if you need to do that.)
[BTW, you might think these popular forward declarations should be in
a header of their own, to avoid some repetition. That's a reasonable
idea, and there is a file jspubtd.h that serves this purpose... sort
of. It contains numerous forward declarations (many more than is
needed in 98% of cases), but it also defines some types, so its
purpose is muddied. So I'm not doing that for now, though creating a
new file js/ForwardDecls.h (or somesuch) is a possibility in the
future.]
As well as using forward declarations where suitable, I've started
breaking off small sections of jsapi.h into separate headers in
js/public/. Unfortunately, this work will only go so far because
xpcpublic.h, BindingUtils.h, and DOMJSClass.h all (unavoidably)
include jsapi.h, and they are headers that are large and included in
lots of places. I'd love to hear suggestions as to how they could be
broken into smaller pieces and/or included in fewer places.
Issues like this could be identified and prevented in the future if we
had static analysis identifying badness. Is bug 887641 the only thing
blocking unhiding the static analysis builders on TBPL (887642)? Has
anyone filed bugs to get #include sanity checking added to our Clang
plugin?
In order to do that we would basically need to build a bug-free
include-what-you-use, and AFAIK nobody has signed up to do that work.
Also note that such a check will only be useful if we adhere to the
principle of using forward-decls where possible throughout the tree,
otherwise there will be thousands of known failures. We're very far
from that right now.
Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Stop #including jsapi.h everywhere!
On 8/19/13 8:15 PM, Nicholas Nethercote wrote: Unfortunately, this work will only go so far because xpcpublic.h, BindingUtils.h, and DOMJSClass.h all (unavoidably) include jsapi.h, and they are headers that are large and included in lots of places. I'd love to hear suggestions as to how they could be broken into smaller pieces and/or included in fewer places. DOMJSClass.h only needs various forward-declarations, mostly. The exceptions are: 1) It needs js::GetGlobalForObjectCrossCompartment (used in an inline method), but we could move that bit into some other header. The declaration of the inline method would need to stay here, but the definition could be somewhere else. 2) It needs the definition of JSClass, for a member of the DOMJSClass struct and the DOMIfaceAndProtoJSClass struct. Unfortunately, this is defined in jsapi.h and defining the DOMJSClass struct is the main point of this header file. 3) It needs js::GetReservedSlot for some inline functions, but we could put those in some other header. BindingUtils.h we could try breaking up in various ways, but it should be very rare for _headers_ to include that file; for the most part such inclusions are a bug from my point of view. For non-headers that include it (e.g. binding implementation files), it might well be common to need jsapi.h. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Stop #including jsapi.h everywhere!
On 8/20/13 7:27 AM, Ehsan Akhgari wrote:
On 2013-08-19 9:10 PM, Gregory Szorc wrote:
On 8/19/13 5:15 PM, Nicholas Nethercote wrote:
Hi,
Analysis in https://bugzilla.mozilla.org/show_bug.cgi?id=901132 has
indicated that jsapi.h is probably responsible for more recompilation
than any other file in the Mozilla tree -- it gets included in *lots*
of files, and it gets modified very often, partly because it's so
large. (jsfriendapi.h is also bad in this respect.)
I'm trying to improve this situation in
https://bugzilla.mozilla.org/show_bug.cgi?id=905017. It turns out
that *many* |#include jsapi.h| statements are simply not needed.
They can be replaced by forward declarations of a handful of types,
e.g.:
struct jsid;
struct JSContext;
class JSObject;
namespace JS {
template typename T class Handle;
template typename T class MutableHandle;
class Value; // #include js/Value.h (not jsapi.h) if you need the
*definition*
}
Next time you're thinking of adding a |#include jsapi.h| statement,
please think about whether a forward declaration would suffice -- i.e.
if you are only using public JS types (i.e. not functions), and only
using them as pointers, references, or parameters in function
declarations. (Forward-declaring JS_PUBLIC_API types is harder; ask
me for help if you need to do that.)
[BTW, you might think these popular forward declarations should be in
a header of their own, to avoid some repetition. That's a reasonable
idea, and there is a file jspubtd.h that serves this purpose... sort
of. It contains numerous forward declarations (many more than is
needed in 98% of cases), but it also defines some types, so its
purpose is muddied. So I'm not doing that for now, though creating a
new file js/ForwardDecls.h (or somesuch) is a possibility in the
future.]
As well as using forward declarations where suitable, I've started
breaking off small sections of jsapi.h into separate headers in
js/public/. Unfortunately, this work will only go so far because
xpcpublic.h, BindingUtils.h, and DOMJSClass.h all (unavoidably)
include jsapi.h, and they are headers that are large and included in
lots of places. I'd love to hear suggestions as to how they could be
broken into smaller pieces and/or included in fewer places.
Issues like this could be identified and prevented in the future if we
had static analysis identifying badness. Is bug 887641 the only thing
blocking unhiding the static analysis builders on TBPL (887642)? Has
anyone filed bugs to get #include sanity checking added to our Clang
plugin?
In order to do that we would basically need to build a bug-free
include-what-you-use, and AFAIK nobody has signed up to do that work.
Also note that such a check will only be useful if we adhere to the
principle of using forward-decls where possible throughout the tree,
otherwise there will be thousands of known failures. We're very far
from that right now.
I'm not talking about building a bug-free IWYU: I'm talking about making
the static analyzer be able to quickly identify suboptimal foo. e.g. we
would maintain a list of please forward declare symbols from jsapi.h.
At compile time if a file includes jsapi.h and only uses symbols from
the please forward declare list, then a warning is issued. We can put
the additional checks behind compiler flags/warnings and can ratchet up
the strictness over time. How will this not work?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Stop #including jsapi.h everywhere!
On 08/20/2013 09:01 AM, Boris Zbarsky wrote: DOMJSClass.h only needs various forward-declarations, mostly. The exceptions are: 2) It needs the definition of JSClass, for a member of the DOMJSClass struct and the DOMIfaceAndProtoJSClass struct. Unfortunately, this is defined in jsapi.h and defining the DOMJSClass struct is the main point of this header file. We can/should add js/public/Class.h and move all the class gunk into that. I think the only dependencies of JSClass are various JSCLASS_* macros and the function typedefs, and those only depend on the rooting types and JSAPI structs that we can forward-declare. Jeff ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Stop #including jsapi.h everywhere!
On 2013-08-20 12:52 PM, Gregory Szorc wrote: In order to do that we would basically need to build a bug-free include-what-you-use, and AFAIK nobody has signed up to do that work. Also note that such a check will only be useful if we adhere to the principle of using forward-decls where possible throughout the tree, otherwise there will be thousands of known failures. We're very far from that right now. I'm not talking about building a bug-free IWYU: I'm talking about making the static analyzer be able to quickly identify suboptimal foo. e.g. we would maintain a list of please forward declare symbols from jsapi.h. At compile time if a file includes jsapi.h and only uses symbols from the please forward declare list, then a warning is issued. We can put the additional checks behind compiler flags/warnings and can ratchet up the strictness over time. How will this not work? Yeah, we can probably make it work for a small subset of things, since we wouldn't need to address all imaginable cases for them. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Viewing resource usage of builds
Now in inbound, |mach build| will record *system* resource usage during builds. You can later view the resource usage by running |mach resource-usage| (mach will tell you at the end of the build if you can run this). The goal of this feature is to help people diagnose why builds are slow and how builds are changing over time. You should be able to easily see where CPU isn't at 100%, when I/O is slowing you down, when you are swapping, etc. We should eventually be able to use this information to tell you things like your build could be faster if you had an SSD, more memory, or more CPU cores. (We previously rolled out similar resource monitoring to mozharness and hope to one day have builds and automation producing the same machine readable data format so we can reuse the HTML viewer to easily view resource usage of many different kinds of jobs. The goal here is to identify poor hardware resource efficiency so we can improve it.) We only achieved the initial milestone by landing this feature. We still have a lot of work to do. Bug 901601 tracks enabling this on non-mach builds, which will start recording resource usage on automation infrastructure. Bug 907297 tracks making the HTML display better. Bug 892342 tracks making the resource monitor more accurate. These will all hopefully land in due time. If you notice anything weird, please raise it in bug 883209 if it is critical or file a follow-up in Core :: Build Config. The HTML display is currently pretty poor. I'm not a good web developer :) The HTML display is not part of the build, so I will rubber stamp pretty much any patch that improves it. Please scratch your own itches! Bug 907297 contains a list of suggested improvements. For comparison purposes, [1] contains the output from a clobber build with empty ccache on my MBP. As you can see, there are large chunks of the build where we aren't CPU bound. Many of these are during recursive make traversal, which is one reason we're focusing on moving away from recursive make. Finally, this is just a friendly reminder that if you build mozilla-central and you don't have a machine with at least 4 *physical* cores, 16 GB RAM, and an SSD, you should upgrade your hardware. [1] https://people.mozilla.com/~gszorc/resources-references.png Gregory ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
MemShrink meeting: Today, August 20 2013 @ 4:00pm PDT
The next MemShrink meeting will be brought to you by bug 907201: Enable Type Inference and IonMonkey for chrome code The wiki page for this meeting is at: https://wiki.mozilla.org/Performance/MemShrink Agenda: * Prioritize unprioritized MemShrink bugs. * Discuss how we measure progress. * Discuss approaches to getting more data. Meeting details: * Tue, 20 August, 4:00 PM PDT * Vidyo: SFO 7N Noise Pop * MTV: Very Good Very Mighty, Mountain View office, 3rd floor * SF: Noise Pop. 7th Floor * Dial-in Info: - In office or soft phone: extension 92 - US/INTL: 650-903-0800 or 650-215-1282 then extension 92 - Toll-free: 800-707-2533 then password 369 - Conference num 95704 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Stop #including jsapi.h everywhere!
On Tue, Aug 20, 2013 at 11:19 AM, Jeff Walden jwalden+...@mit.edu wrote: On 08/20/2013 09:01 AM, Boris Zbarsky wrote: DOMJSClass.h only needs various forward-declarations, mostly. The exceptions are: 2) It needs the definition of JSClass, for a member of the DOMJSClass struct and the DOMIfaceAndProtoJSClass struct. Unfortunately, this is defined in jsapi.h and defining the DOMJSClass struct is the main point of this header file. We can/should add js/public/Class.h and move all the class gunk into that. I think the only dependencies of JSClass are various JSCLASS_* macros and the function typedefs, and those only depend on the rooting types and JSAPI structs that we can forward-declare. There's a bunch of stuff in jsapi.h that will need to be moved into Class.h, but it's quite doable and on my short-term todo list. I might be able to remove jsfriendapi.h's dependency on jsapi.h as a result of this as well. Nick ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
