code review 37690045: os: avoid a string concat in readdir

os: avoid a string concat in readdir

[bradfitz, 2013-12-17 20:21:41]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[crawshaw1, 2013-12-17 20:24:19]

LGTM

[bradfitz, 2013-12-17 20:25:34]

*** Submitted as https://code.google.com/p/go/source/detail?r=455c7e94abfc ***

os: avoid a string concat in readdir

R=golang-dev, crawshaw
CC=golang-dev
https://codereview.appspot.com/37690045

[dave_cheney.net, 2013-12-17 21:08:03]

NOT LGTM.

How is this better? You've gone from dirname += "/" outside the loop
to x := dirname + "/"; x += filename inside the loop.

On Wed, Dec 18, 2013 at 7:25 AM,  <bradfitz@golang.org> wrote:
> *** Submitted as
> https://code.google.com/p/go/source/detail?r=455c7e94abfc ***
>
>
> os: avoid a string concat in readdir
>
> R=golang-dev, crawshaw
> CC=golang-dev
> https://codereview.appspot.com/37690045
>
>
>
> https://codereview.appspot.com/37690045/
>
> --
>
> ---You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.

[iant, 2013-12-17 21:12:25]

On Tue, Dec 17, 2013 at 1:08 PM, Dave Cheney <dave@cheney.net> wrote:
>
> How is this better? You've gone from dirname += "/" outside the loop
> to x := dirname + "/"; x += filename inside the loop.

The gc compiler optimizes a + b + c into a single operation: in this
case it will generate code that passes all three strings to
concatstring3.

Ian

[bradfitz, 2013-12-17 21:14:14]

On Tue, Dec 17, 2013 at 1:12 PM, Ian Lance Taylor <iant@golang.org> wrote:

> On Tue, Dec 17, 2013 at 1:08 PM, Dave Cheney <dave@cheney.net> wrote:
> >
> > How is this better? You've gone from dirname += "/" outside the loop
> > to x := dirname + "/"; x += filename inside the loop.
>
> The gc compiler optimizes a + b + c into a single operation: in this
> case it will generate code that passes all three strings to
> concatstring3.
>

Yeah. But what I forgot to consider (and don't know) was whether gccgo does
that too.

[iant, 2013-12-17 21:57:39]

On Tue, Dec 17, 2013 at 1:14 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> On Tue, Dec 17, 2013 at 1:12 PM, Ian Lance Taylor <iant@golang.org> wrote:
>>
>> On Tue, Dec 17, 2013 at 1:08 PM, Dave Cheney <dave@cheney.net> wrote:
>> >
>> > How is this better? You've gone from dirname += "/" outside the loop
>> > to x := dirname + "/"; x += filename inside the loop.
>>
>> The gc compiler optimizes a + b + c into a single operation: in this
>> case it will generate code that passes all three strings to
>> concatstring3.
>
>
> Yeah. But what I forgot to consider (and don't know) was whether gccgo does
> that too.

It doesn't.  But it's easy.

Ian

[dave_cheney.net, 2013-12-17 22:09:33]

Thanks, sorry for the noise. I'll check my facts next time. 

> On 18 Dec 2013, at 8:57, Ian Lance Taylor <iant@golang.org> wrote:
> 
>> On Tue, Dec 17, 2013 at 1:14 PM, Brad Fitzpatrick <bradfitz@golang.org>
wrote:
>>> On Tue, Dec 17, 2013 at 1:12 PM, Ian Lance Taylor <iant@golang.org> wrote:
>>> 
>>>> On Tue, Dec 17, 2013 at 1:08 PM, Dave Cheney <dave@cheney.net> wrote:
>>>> 
>>>> How is this better? You've gone from dirname += "/" outside the loop
>>>> to x := dirname + "/"; x += filename inside the loop.
>>> 
>>> The gc compiler optimizes a + b + c into a single operation: in this
>>> case it will generate code that passes all three strings to
>>> concatstring3.
>> 
>> 
>> Yeah. But what I forgot to consider (and don't know) was whether gccgo does
>> that too.
> 
> It doesn't.  But it's easy.
> 
> Ian