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

2013-09-12 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 algorithm 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 files? how many other files

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

2013-09-12 Thread Benoit Jacob
2013/9/12 Avi Hal avi...@gmail.com 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 algorithm just to get std::min and std::max. Is it because min/max are used at the h file? Yes. can it

Re: Including algorithm 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 avi...@gmail.com 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 algorithm just to get std::min and

Re: Including algorithm 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

Re: Including algorithm 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 algorithm 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

Re: Including algorithm 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 move

Re: Including algorithm 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

Re: Including algorithm 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

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

2013-09-08 Thread Nicholas Cameron
I timed builds to see if this makes a significant difference and it did not. I timed a clobber debug build using clang with no ccache on Linux on a fast laptop. I timed using a pull from m-c about a week old (I am using this pull because I have a lot of other stats on it). I then applied

Re: Including algorithm 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 nick.r.came...@gmail.com 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

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

2013-09-08 Thread Benoit Jacob
We have many other headers including algorithm; it would be interesting to compare the percentage of our cpp files that recursively include algorithm before and after that patch; I suppose that just a single patch like that is not enough to move that needle much, because there are other ways that

Re: Including algorithm 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 algorithm; it would be interesting to compare the percentage of our cpp files that recursively include algorithm before and after that patch; I suppose that just a single patch like that is not

Re: Including algorithm 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

Re: Including algorithm 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 algorithm; it would be interesting to compare the percentage of our cpp files that recursively include algorithm before and after

Re: Including algorithm 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