base::Bind: Increase the arity to 7.

base::Bind: Increase the arity to 7.

This is required by a CL I have in the works.

Stats from builds of Release Chrome target with -j12:
Build time: -0.2%, within the noise. 
Size: No change in size of chrome binary.

BUG=98542
TEST=none

R=ajwong@chromium.org,akalin@chromium.org,willchan@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=105754

[willchan no longer on Chromium, 2011-10-15 23:55:10]

We discussed this recently. Do you really need this? Does using a struct not
work? That's what we've done elsewhere and in the IPC messages too.

On Sat, Oct 15, 2011 at 4:36 PM, <jhawkins@chromium.org> wrote:

> Reviewers: awong, akalin, willchan,
>
> Description:
> base::Bind: Increase the arity to 7.
>
> This is required by a CL I have in the works.
>
> Stats from builds of Release Chrome target:
> Build time: +0.312s (50.951s average -> 51.260s average), +0.6%
> Size: No change in size of chrome binary.
>
> BUG=98542
> TEST=none
>
> R=ajwong@chromium.org,akalin@**chromium.org <akalin@chromium.org>,
> willchan@**chromium.org <willchan@chromium.org>
>
>
> Please review this at
http://codereview.chromium.**org/8306008/<http://codereview.chromium.org/8306...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M base/bind.h
>  M base/bind.h.pump
>  M base/bind_internal.h
>  M base/bind_internal.h.pump
>
>
>

[James Hawkins, 2011-10-16 00:48:41]

On 2011/10/15 23:55:10, willchan wrote:
> We discussed this recently. Do you really need this? Does using a struct not
> work? That's what we've done elsewhere and in the IPC messages too.
> 

For the record, I discussed this with Albert yesterday and he approved; he asked
me to add you and akalin to the R since he'll likely be OOO (or something).

I prefer this solution to changing the call signature especially since the time
increase, which even if outside the noise, is not significant.

[willchan no longer on Chromium, 2011-10-16 02:10:08]

Interesting numbers. Do you believe they hold up for Windows builds too? I
assume you are using GOMA, so the minor increase in compile-time of
individual translation units is more dominated by network latency to the
various compile slaves. On the Windows builders, that is not the case, since
those are single heavy-duty machines, so my suspicion (but I admittedly
don't really know), is that times will increase more.

And how many call sites have we converted over to base::Bind() now? 1/3 of
them? Will the build slowdown increase linearly with an increase in
call-sites?

0.6% is indeed negligible in my opinion. If you believe (and even better,
can show) this is the expected slowdown for all our builders (most important
Windows), then I'm cool with this. Also, I'd be willing to let you commit
this as is and then observe the Windows clobber builders to see if there's a
noticeable impact (and revert if so). Times seem to be 47-51 minutes, but
mostly 49 minutes. I'm basing this off:

http://build.chromium.org/p/chromium/builders/Win
http://build.chromium.org/p/chromium/builders/Win/builds/7481 - 51
http://build.chromium.org/p/chromium/builders/Win/builds/7480 - 49
http://build.chromium.org/p/chromium/builders/Win/builds/7479 - 47
http://build.chromium.org/p/chromium/builders/Win/builds/7478 - 49
http://build.chromium.org/p/chromium/builders/Win/builds/7477 - 49

Thoughts?

On Sat, Oct 15, 2011 at 5:48 PM, <jhawkins@chromium.org> wrote:

> On 2011/10/15 23:55:10, willchan wrote:
>
>> We discussed this recently. Do you really need this? Does using a struct
>> not
>> work? That's what we've done elsewhere and in the IPC messages too.
>>
>
>
> For the record, I discussed this with Albert yesterday and he approved; he
> asked
> me to add you and akalin to the R since he'll likely be OOO (or something).
>

Yeah, he's out for 2 weeks.


> I prefer this solution to changing the call signature especially since the
> time
> increase, which even if outside the noise, is not significant.
>
>
http://codereview.chromium.**org/8306008/<http://codereview.chromium.org/8306...
>

[James Hawkins, 2011-10-16 02:38:53]

Yeah, I'm not sure about what the Windows build time diff will be, but I'm fine
with committing and getting numbers from the bot cycle times.

For the test, I used our local distcc farm, not goma, but I agree that still
introduces too much dependence on the network as a variable; I'll test again
with a local-only build to get rid of the network variable.

We've converted ~43% of the callsites; I suspect the increase will be linear so
whatever increase we measure will likely double (I think).

On 2011/10/16 02:10:08, willchan wrote:
> Interesting numbers. Do you believe they hold up for Windows builds too? I
> assume you are using GOMA, so the minor increase in compile-time of
> individual translation units is more dominated by network latency to the
> various compile slaves. On the Windows builders, that is not the case, since
> those are single heavy-duty machines, so my suspicion (but I admittedly
> don't really know), is that times will increase more.
> 
> And how many call sites have we converted over to base::Bind() now? 1/3 of
> them? Will the build slowdown increase linearly with an increase in
> call-sites?
> 
> 0.6% is indeed negligible in my opinion. If you believe (and even better,
> can show) this is the expected slowdown for all our builders (most important
> Windows), then I'm cool with this. Also, I'd be willing to let you commit
> this as is and then observe the Windows clobber builders to see if there's a
> noticeable impact (and revert if so). Times seem to be 47-51 minutes, but
> mostly 49 minutes. I'm basing this off:
> 
> http://build.chromium.org/p/chromium/builders/Win
> http://build.chromium.org/p/chromium/builders/Win/builds/7481 - 51
> http://build.chromium.org/p/chromium/builders/Win/builds/7480 - 49
> http://build.chromium.org/p/chromium/builders/Win/builds/7479 - 47
> http://build.chromium.org/p/chromium/builders/Win/builds/7478 - 49
> http://build.chromium.org/p/chromium/builders/Win/builds/7477 - 49
> 
> Thoughts?
> 
> On Sat, Oct 15, 2011 at 5:48 PM, <mailto:jhawkins@chromium.org> wrote:
> 
> > On 2011/10/15 23:55:10, willchan wrote:
> >
> >> We discussed this recently. Do you really need this? Does using a struct
> >> not
> >> work? That's what we've done elsewhere and in the IPC messages too.
> >>
> >
> >
> > For the record, I discussed this with Albert yesterday and he approved; he
> > asked
> > me to add you and akalin to the R since he'll likely be OOO (or something).
> >
> 
> Yeah, he's out for 2 weeks.
> 
> 
> > I prefer this solution to changing the call signature especially since the
> > time
> > increase, which even if outside the noise, is not significant.
> >
> >
>
http://codereview.chromium.**org/8306008/%3Chttp://codereview.chromium.org/83...>
> >

[willchan no longer on Chromium, 2011-10-16 02:41:12]

Sounds great. LGTM. Feel free to land this to test on the bots.

On Sat, Oct 15, 2011 at 7:38 PM, <jhawkins@chromium.org> wrote:

> Yeah, I'm not sure about what the Windows build time diff will be, but I'm
> fine
> with committing and getting numbers from the bot cycle times.
>
> For the test, I used our local distcc farm, not goma, but I agree that
> still
> introduces too much dependence on the network as a variable; I'll test
> again
> with a local-only build to get rid of the network variable.
>
> We've converted ~43% of the callsites; I suspect the increase will be
> linear so
> whatever increase we measure will likely double (I think).
>
>
> On 2011/10/16 02:10:08, willchan wrote:
>
>> Interesting numbers. Do you believe they hold up for Windows builds too? I
>> assume you are using GOMA, so the minor increase in compile-time of
>> individual translation units is more dominated by network latency to the
>> various compile slaves. On the Windows builders, that is not the case,
>> since
>> those are single heavy-duty machines, so my suspicion (but I admittedly
>> don't really know), is that times will increase more.
>>
>
>  And how many call sites have we converted over to base::Bind() now? 1/3 of
>> them? Will the build slowdown increase linearly with an increase in
>> call-sites?
>>
>
>  0.6% is indeed negligible in my opinion. If you believe (and even better,
>> can show) this is the expected slowdown for all our builders (most
>> important
>> Windows), then I'm cool with this. Also, I'd be willing to let you commit
>> this as is and then observe the Windows clobber builders to see if there's
>> a
>> noticeable impact (and revert if so). Times seem to be 47-51 minutes, but
>> mostly 49 minutes. I'm basing this off:
>>
>
> 
http://build.chromium.org/p/**chromium/builders/Win<http://build.chromium.org...
>>
http://build.chromium.org/p/**chromium/builders/Win/builds/**7481<http://buil...
51
>>
http://build.chromium.org/p/**chromium/builders/Win/builds/**7480<http://buil...
49
>>
http://build.chromium.org/p/**chromium/builders/Win/builds/**7479<http://buil...
47
>>
http://build.chromium.org/p/**chromium/builders/Win/builds/**7478<http://buil...
49
>>
http://build.chromium.org/p/**chromium/builders/Win/builds/**7477<http://buil...
49
>>
>
>  Thoughts?
>>
>
>  On Sat, Oct 15, 2011 at 5:48 PM, <mailto:jhawkins@chromium.org> wrote:
>>
>
>  > On 2011/10/15 23:55:10, willchan wrote:
>> >
>> >> We discussed this recently. Do you really need this? Does using a
>> struct
>> >> not
>> >> work? That's what we've done elsewhere and in the IPC messages too.
>> >>
>> >
>> >
>> > For the record, I discussed this with Albert yesterday and he approved;
>> he
>> > asked
>> > me to add you and akalin to the R since he'll likely be OOO (or
>> something).
>> >
>>
>
>  Yeah, he's out for 2 weeks.
>>
>
>
>  > I prefer this solution to changing the call signature especially since
>> the
>> > time
>> > increase, which even if outside the noise, is not significant.
>> >
>> >
>>
>
> http://codereview.chromium.****org/8306008/%3Chttp://coderevi**
> ew.chromium.org/8306008/ <http://codereview.chromium.org/8306008/>>
>
>> >
>>
>
>
>
>
http://codereview.chromium.**org/8306008/<http://codereview.chromium.org/8306...
>

[awong, 2011-10-16 03:10:50]

(still here for another few hours...)

Sounds like a good plan.  To clarify, my only 2 concerns with more arity
are:

  1) build time
  2) file size

If those stay sane, we should be okay kicking the number upwards.  I think
rsleevi actually wants it at 8 (please reference bug).

So, I'm happy with whatever as long as we know what the build time cost is
and are okay with the hit.

-Albert


On Sat, Oct 15, 2011 at 7:41 PM, William Chan (陈智昌)
<willchan@chromium.org>wrote:

> Sounds great. LGTM. Feel free to land this to test on the bots.
>
>
> On Sat, Oct 15, 2011 at 7:38 PM, <jhawkins@chromium.org> wrote:
>
>> Yeah, I'm not sure about what the Windows build time diff will be, but I'm
>> fine
>> with committing and getting numbers from the bot cycle times.
>>
>> For the test, I used our local distcc farm, not goma, but I agree that
>> still
>> introduces too much dependence on the network as a variable; I'll test
>> again
>> with a local-only build to get rid of the network variable.
>>
>> We've converted ~43% of the callsites; I suspect the increase will be
>> linear so
>> whatever increase we measure will likely double (I think).
>>
>>
>> On 2011/10/16 02:10:08, willchan wrote:
>>
>>> Interesting numbers. Do you believe they hold up for Windows builds too?
>>> I
>>> assume you are using GOMA, so the minor increase in compile-time of
>>> individual translation units is more dominated by network latency to the
>>> various compile slaves. On the Windows builders, that is not the case,
>>> since
>>> those are single heavy-duty machines, so my suspicion (but I admittedly
>>> don't really know), is that times will increase more.
>>>
>>
>>  And how many call sites have we converted over to base::Bind() now? 1/3
>>> of
>>> them? Will the build slowdown increase linearly with an increase in
>>> call-sites?
>>>
>>
>>  0.6% is indeed negligible in my opinion. If you believe (and even better,
>>> can show) this is the expected slowdown for all our builders (most
>>> important
>>> Windows), then I'm cool with this. Also, I'd be willing to let you commit
>>> this as is and then observe the Windows clobber builders to see if
>>> there's a
>>> noticeable impact (and revert if so). Times seem to be 47-51 minutes, but
>>> mostly 49 minutes. I'm basing this off:
>>>
>>
>> 
http://build.chromium.org/p/**chromium/builders/Win<http://build.chromium.org...
>>>
http://build.chromium.org/p/**chromium/builders/Win/builds/**7481<http://buil...
51
>>>
http://build.chromium.org/p/**chromium/builders/Win/builds/**7480<http://buil...
49
>>>
http://build.chromium.org/p/**chromium/builders/Win/builds/**7479<http://buil...
47
>>>
http://build.chromium.org/p/**chromium/builders/Win/builds/**7478<http://buil...
49
>>>
http://build.chromium.org/p/**chromium/builders/Win/builds/**7477<http://buil...
49
>>>
>>
>>  Thoughts?
>>>
>>
>>  On Sat, Oct 15, 2011 at 5:48 PM, <mailto:jhawkins@chromium.org> wrote:
>>>
>>
>>  > On 2011/10/15 23:55:10, willchan wrote:
>>> >
>>> >> We discussed this recently. Do you really need this? Does using a
>>> struct
>>> >> not
>>> >> work? That's what we've done elsewhere and in the IPC messages too.
>>> >>
>>> >
>>> >
>>> > For the record, I discussed this with Albert yesterday and he approved;
>>> he
>>> > asked
>>> > me to add you and akalin to the R since he'll likely be OOO (or
>>> something).
>>> >
>>>
>>
>>  Yeah, he's out for 2 weeks.
>>>
>>
>>
>>  > I prefer this solution to changing the call signature especially since
>>> the
>>> > time
>>> > increase, which even if outside the noise, is not significant.
>>> >
>>> >
>>>
>>
>> http://codereview.chromium.****org/8306008/%3Chttp://coderevi**
>> ew.chromium.org/8306008/ <http://codereview.chromium.org/8306008/>>
>>
>>> >
>>>
>>
>>
>>
>>
http://codereview.chromium.**org/8306008/<http://codereview.chromium.org/8306...
>>
>
>

[James Hawkins, 2011-10-16 22:42:34]

I tested with a local-only build with -j12.  Compile time actually decreased
0.2%, which is further evidence the change is in the noise.  I'm going to commit
and watch the Win bot build times.