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

2013-09-12 Thread Trevor Saunders
On Thu, Sep 12, 2013 at 11:59:30PM +0200, Julian Seward wrote: > On 09/12/2013 11:08 PM, Chris Peterson wrote: > > On 9/12/13 6:35 AM, Mike Hommey wrote: > >> Note we have *many* inline functions that the compiler decide to never > >> inline. We should maybe try to detect those on all platforms and

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

2013-09-12 Thread Nicholas Cameron
I suppose that that metric will be different between compilers (msvc vs gcc vs clang (which we don't officially build with, but I bet is the easiest to get the info out of)), and possibly between platforms, versions, etc. I wouldn't be surprised if the context in which the header is included mak

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

2013-09-12 Thread Julian Seward
On 09/12/2013 11:08 PM, Chris Peterson wrote: > On 9/12/13 6:35 AM, Mike Hommey wrote: >> Note we have *many* inline functions that the compiler decide to never >> inline. We should maybe try to detect those on all platforms and move >> those functions out of headers. > > gcc -Winline will report

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

2013-09-12 Thread Chris Peterson
On 9/12/13 6:35 AM, Mike Hommey wrote: Note we have *many* inline functions that the compiler decide to never inline. We should maybe try to detect those on all platforms and move those functions out of headers. gcc -Winline will report uninlined "inline" functions, but the warnings are VERY n

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

2013-09-12 Thread Mike Hommey
On Thu, Sep 12, 2013 at 07:19:54AM -0400, Benoit Jacob wrote: > 2013/9/12 Avi Hal > > > On Sunday, September 8, 2013 6:22:01 AM UTC+3, Benoit Jacob wrote: > > > Hi, > > > > > > > > > > > > It seems that we have some much-included header

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

2013-09-12 Thread Benoit Jacob
2013/9/12 Avi Hal > On Sunday, September 8, 2013 6:22:01 AM UTC+3, Benoit Jacob wrote: > > Hi, > > > > > > > > It seems that we have some much-included header files including > > > > > just to get std::min and std::max. > > > > Is

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

2013-09-11 Thread Avi Hal
On Sunday, September 8, 2013 6:22:01 AM UTC+3, Benoit Jacob wrote: > Hi, > > > > It seems that we have some much-included header files including > > just to get std::min and std::max. > Is it because min/max are used at the h file? can it be delegated to cpp file

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

2013-09-09 Thread Dao
On 09.09.2013 03:21, Benoit Jacob wrote: Again, how many other similar wins are we leaving on the table because they're only 10s on a clobber build? It's of course hard to know, which is why I've suggested the (number of useful lines of code) / (total lines of code included) ratio as a meaningful

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

2013-09-09 Thread Jonathan Kew
On 9/9/13 00:29, Nicholas Cameron wrote: I don't think these kind of time improvements make it worth duplicating std library code into mfbt, we may as well just pull in the headers and forget about it. +1 to that. We've been trying to get *away* from requiring special Mozilla-isms in our cod

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

2013-09-08 Thread Boris Zbarsky
On 9/8/13 7:29 PM, Nicholas Cameron wrote: I timed builds to see if this makes a significant difference and it did not.. The other thing that reducing .i size helps is Windows PGO memory usage. See graph at http://graphs.mozilla.org/graph.html#tests=[[205,63,8]]&sel=none&displayrange=90&data

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

2013-09-08 Thread Mike Hommey
On Mon, Sep 09, 2013 at 10:12:35AM +0900, Mike Hommey wrote: > On Sun, Sep 08, 2013 at 08:52:23PM -0400, Benoit Jacob wrote: > > We have many other headers including ; it would be interesting > > to compare the percentage of our cpp files that recursively include > > before and after that patch; I

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

2013-09-08 Thread Benoit Jacob
Again, how many other similar wins are we leaving on the table because they're only 10s on a clobber build? It's of course hard to know, which is why I've suggested the (number of useful lines of code) / (total lines of code included) ratio as a meaningful metric. But I'm completely OK with focusi

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

2013-09-08 Thread Mike Hommey
On Sun, Sep 08, 2013 at 08:52:23PM -0400, Benoit Jacob wrote: > We have many other headers including ; it would be interesting > to compare the percentage of our cpp files that recursively include > before and after that patch; I suppose that just a single patch > like that is not enough to move t

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

2013-09-08 Thread Benoit Jacob
makes me wonder exactly what aspect of > header inclusion (if not size, which we should catch here) makes the > difference. > > Nick. > > On Sunday, September 8, 2013 3:22:01 PM UTC+12, Benoit Jacob wrote: > > Hi, > > > > > > > > It seems that we have

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

2013-09-08 Thread Nicholas Nethercote
On Sun, Sep 8, 2013 at 4:29 PM, Nicholas Cameron wrote: > > I don't think these kind of time improvements make it worth duplicating std > library code into mfbt, we may as well just pull in the headers and forget > about it. A caveat would be if it makes a significant difference on slower > sys

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

2013-09-08 Thread Nicholas Cameron
ay, September 8, 2013 3:22:01 PM UTC+12, Benoit Jacob wrote: > 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 inclu

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