Including just to get std::min and std::max

2013-09-07 Thread Benoit Jacob 
Hi,

It seems that we have some much-included header files including 
just to get std::min and std::max.

That seems like an extreme case of low ratio between lines of code included
(9,290 on my system, see Appendix below) and lines of code actually used
(say 6 with whitespace).

I ran into this issue while trying to minimize nsCoord.h (
https://bugzilla.mozilla.org/show_bug.cgi?id=913868 ) and in my patch, I
resorted to defining my own min/max functions in a nsCoords_details
namespace.

This prompted comments on that bug suggesting that it might be better to
have that in MFBT. But that, in turn, sounds like overturning our recent
decision to switch to std::min / std::max, which I feel is material for
this mailing list.

It is also conceivable to keep saying that we should use std::min /
std::max *except* in headers that don't otherwise include ,
where it may be more reasonable to use the cheap-to-#include variant
instead.

What do you think?

Benoit

=== Appendix: how big and long to compile is  ? ===

On my Ubuntu 12.04 64bit system, with GCC 4.6.3, including 
means recursively including 9,290 lines of code:

$ echo '#include' > a.cpp && g++ -save-temps -c a.cpp && wc -l
a.ii
9290 a.ii

On may wonder what this implies in terms of compilation times; here is a
naive answer. I'm timing 10 successive compilations of a file that just
includes , and then I do the same with a file that also includes
.

$ echo '#include' > a.cpp && time (g++ -c a.cpp && g++ -c a.cpp
&& g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++ -c
a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp)

real0m1.391s
user0m1.108s
sys 0m0.212s

echo '#include' > a.cpp && echo '#include' >> a.cpp &&
time (g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++
-c a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp && g++ -c a.cpp &&
g++ -c a.cpp)

real0m1.617s
user0m1.324s
sys 0m0.244s

(I actually repeated this many times and kept the best result for each; my
hardware is a Thinkpad W520 with a 2.5GHz, 8M cache Core i7).

So we see that adding the #include made each compilation 23 ms
longer in average (226 ms for 10 compilations).
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Stop #including jsapi.h everywhere!

2013-09-07 Thread Boris Zbarsky 

On 9/7/13 9:43 PM, Nicholas Nethercote wrote:

One possibility is to move them into their own header(s) in
js/public/


That would work, yes.


 Another is to heap-allocate them


This has somewhat undesirable performance implications, not to mention 
the general weirdness of heap-allocating RAII classes.


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Stop #including jsapi.h everywhere!

2013-09-07 Thread Nicholas Nethercote 
On Sun, Sep 8, 2013 at 11:02 AM, Boris Zbarsky  wrote:
>
> I believe nsCxPusher.h only needs jsapi.h because it needs to know
> sizeof(JSAutoRequest) and sizeof(JSAutoCompartment) for the members of
> AutoCxPusher...  Not sure what we can do with that.

One possibility is to move them into their own header(s) in
js/public/.  Another is to heap-allocate them, so we'd just need
forward declarations in nsCxPusher.h.

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Creating mock nsIPrintingPromptService

2013-09-07 Thread Sunil Agrawal 
(Apologies if this is not the right forum, in which case please direct me to 
one).

I am looking at creating a mock nsIPrintingPromptService (so that I can bring 
up my own Print dialog) for my testing purpose, preferably in Javascript.

I looked around Mozilla code base but couldn't find a starting point. Is there 
an existing test that could be doing something similar that I can use?

Thanks in advance,
Sunil 
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Stop #including jsapi.h everywhere!

2013-09-07 Thread Boris Zbarsky 

On 9/7/13 8:49 PM, Nicholas Nethercote wrote:

I've been focusing entirely on jsapi.h includes.  We're down to ~1480
rebuilt files when it gets touched;  we started at ~2600.  It's
getting hard to improve, because there's a tangled clump of
widely-included files that include it still either directly or
indirectly:  BindingUtils.h, xpcpublic.h, nsCxPusher.h,
DOMJSProxyHandler.h, Workers.h, probably a couple of others I've
forgotten right now.  For some of these it's not too hard to break the
dependency, but AFAICT we need to break the dependency for most or all
of them, which is harder.


Of those 1480 files, 440 or so are generated binding .cpp files.  You 
can't stop including jsapi.h in those, for obvious reasons.


We should looks at non-binding files that include BindingUtils.h and 
figure out why: I suspect they mostly want UnwrapObject (which 
definitely needs jsapi-ish bits, but could have a non-inline version for 
those callers) or WrapObject (which I expect doesn't need JSAPI for its 
inline parts).


DOMJSProxyHandler.h is included almost entirely in bindings code or via 
BindingUtils.h.


I suspect xpcpublic.h is over-included. 
https://bugzilla.mozilla.org/show_bug.cgi?id=910937 kills off some of 
that, but I bet there's more we can get rid of.


I believe nsCxPusher.h only needs jsapi.h because it needs to know 
sizeof(JSAutoRequest) and sizeof(JSAutoCompartment) for the members of 
AutoCxPusher...  Not sure what we can do with that.


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Stop #including jsapi.h everywhere!

2013-09-07 Thread Nicholas Nethercote 
On Sun, Sep 8, 2013 at 2:38 AM, Benoit Jacob  wrote:
> I just was starting to look at BindingUtils.h as it is one of the most
> important "hub headers" that we have (see
> https://bugzilla.mozilla.org/show_bug.cgi?id=912735). But it seems that you
> guys are already well ahead into BindingUtils.h discussion. Is there a bug
> filed for it?

I've been focusing entirely on jsapi.h includes.  We're down to ~1480
rebuilt files when it gets touched;  we started at ~2600.  It's
getting hard to improve, because there's a tangled clump of
widely-included files that include it still either directly or
indirectly:  BindingUtils.h, xpcpublic.h, nsCxPusher.h,
DOMJSProxyHandler.h, Workers.h, probably a couple of others I've
forgotten right now.  For some of these it's not too hard to break the
dependency, but AFAICT we need to break the dependency for most or all
of them, which is harder.

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Stop #including jsapi.h everywhere!

2013-09-07 Thread Benoit Jacob 
2013/9/7 Benoit Jacob 

>
>
>
> 2013/9/7 Boris Zbarsky 
>
>
>>
>> This is good, but unfortunately  leaks in all over the place
>> anyway in DOM code.
>>
>> The way it does that is that dom/Element.h has inline methods that need
>> nsPresContext.h and Units.h.  Either one will get you things like nsRect.h
>> or nsCoord.h, both of which include .  Oh, nsContentUtils.h
>> includes Units.h too...
>>
>
> Incidentally, nsRect.h just got fixed by
> https://bugzilla.mozilla.org/show_bug.cgi?id=913603
>
> Thanks for pointing out nsCoord.h, let's fix it... (will file a bug and
> block the tracking bug 912735)
>

Bug/patch about nsCoord.h:
https://bugzilla.mozilla.org/show_bug.cgi?id=913868


>
>  If there is a BindingUtils.h tracking bug, they could block it.
>>>
>>
>> There isn't one yet.
>>
>
Filed BindingUtils.h tracking bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=913869

It turns out that a lot of cleanup of BindingUtils.h already just landed:
https://bugzilla.mozilla.org/show_bug.cgi?id=909003

Benoit
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Stop #including jsapi.h everywhere!

2013-09-07 Thread Benoit Jacob 
2013/9/7 Boris Zbarsky 

> On 9/7/13 12:56 PM, Benoit Jacob wrote:
>
>> https://bugzilla.mozilla.org/**show_bug.cgi?id=913847
>>  moves NS_IsMainThread
>> to a new MainThreadUtils.h header that's cheaper to include, and in
>> particular is all what BindingUtils.h needs (there was a helpful comment
>> about that in BindingUtils.h).
>>
>
> Excellent.  Note 
> https://bugzilla.mozilla.org/**show_bug.cgi?id=909971also:
>  we can stop including MainThreadUtils.h in this header too, I think.


Thanks for the link. MainThreadUtils.h is tiny, though, so this won't be a
big deal anymore.


>
>
>  
> https://bugzilla.mozilla.org/**show_bug.cgi?id=913852
>  makes BindingUtils.h
>> not include  just for one use of std::min.
>>
>
> This is good, but unfortunately  leaks in all over the place
> anyway in DOM code.
>
> The way it does that is that dom/Element.h has inline methods that need
> nsPresContext.h and Units.h.  Either one will get you things like nsRect.h
> or nsCoord.h, both of which include .  Oh, nsContentUtils.h
> includes Units.h too...
>

Incidentally, nsRect.h just got fixed by
https://bugzilla.mozilla.org/show_bug.cgi?id=913603

Thanks for pointing out nsCoord.h, let's fix it... (will file a bug and
block the tracking bug 912735)

Benoit


>
> We should strongly consider moving the Element methods that need those
> includes out of line, I think; not sure what we can do about nsContentUtils.
>
>
>  If there is a BindingUtils.h tracking bug, they could block it.
>>
>
> There isn't one yet.
>
> -Boris
>
>
> __**_
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/**listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Stop #including jsapi.h everywhere!

2013-09-07 Thread Boris Zbarsky 

On 9/7/13 12:56 PM, Benoit Jacob wrote:

https://bugzilla.mozilla.org/show_bug.cgi?id=913847  moves NS_IsMainThread
to a new MainThreadUtils.h header that's cheaper to include, and in
particular is all what BindingUtils.h needs (there was a helpful comment
about that in BindingUtils.h).


Excellent.  Note https://bugzilla.mozilla.org/show_bug.cgi?id=909971 
also: we can stop including MainThreadUtils.h in this header too, I think.



https://bugzilla.mozilla.org/show_bug.cgi?id=913852  makes BindingUtils.h
not include  just for one use of std::min.


This is good, but unfortunately  leaks in all over the place 
anyway in DOM code.


The way it does that is that dom/Element.h has inline methods that need 
nsPresContext.h and Units.h.  Either one will get you things like 
nsRect.h or nsCoord.h, both of which include .  Oh, 
nsContentUtils.h includes Units.h too...


We should strongly consider moving the Element methods that need those 
includes out of line, I think; not sure what we can do about nsContentUtils.



If there is a BindingUtils.h tracking bug, they could block it.


There isn't one yet.

-Boris

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Stop #including jsapi.h everywhere!

2013-09-07 Thread Benoit Jacob 
2013/9/7 Benoit Jacob 

> I just was starting to look at BindingUtils.h as it is one of the most
> important "hub headers" that we have (see
> https://bugzilla.mozilla.org/show_bug.cgi?id=912735). But it seems that
> you guys are already well ahead into BindingUtils.h discussion. Is there a
> bug filed for it?
>
> Benoit
>

Here are some patches towards making BindingUtils.h a cheaper header to
include:

https://bugzilla.mozilla.org/show_bug.cgi?id=913847  moves NS_IsMainThread
to a new MainThreadUtils.h header that's cheaper to include, and in
particular is all what BindingUtils.h needs (there was a helpful comment
about that in BindingUtils.h).

https://bugzilla.mozilla.org/show_bug.cgi?id=913852  makes BindingUtils.h
not include  just for one use of std::min.

If there is a BindingUtils.h tracking bug, they could block it.

Benoit
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Stop #including jsapi.h everywhere!

2013-09-07 Thread Benoit Jacob 
I just was starting to look at BindingUtils.h as it is one of the most
important "hub headers" that we have (see
https://bugzilla.mozilla.org/show_bug.cgi?id=912735). But it seems that you
guys are already well ahead into BindingUtils.h discussion. Is there a bug
filed for it?

Benoit


2013/8/21 Nicholas Nethercote 

> On Wed, Aug 21, 2013 at 4:46 PM, Boris Zbarsky  wrote:
> > On 8/21/13 2:23 AM, Nicholas Nethercote wrote:
> >>
> >> And jswrapper.h includes jsapi.h.
>
> I will try to remedy that... it looks doable.
>
> Nick
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform