Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/gc: optimized range over []byte(string) #2204

Closed
bradfitz opened this issue Aug 29, 2011 · 15 comments
Closed

cmd/gc: optimized range over []byte(string) #2204

bradfitz opened this issue Aug 29, 2011 · 15 comments
Labels
FrozenDueToAge GarbageCollector

Comments

@bradfitz
Copy link
Contributor 

A per-byte range over a string (over a []byte(string), not a per-rune range) currently
does a a copy from string->byte, but doesn't need to.

Old mailing list post:

https://groups.google.com/group/golang-dev/browse_thread/thread/a74152dbc459ebba

Low priority, but converting to an issue.

For reference,

func BenchmarkRangeStringLoop(b *testing.B) {
        s := string(make([]byte, 4096))
        for i := 0; i < b.N; i++ {
                for j := 0; j < len(s); j++ {
                        _ = s[j]
                }
        }
}

func BenchmarkRangeStringCast(b *testing.B) {
        s := string(make([]byte, 4096))
        for i := 0; i < b.N; i++ {
                for _, _ = range []byte(s) {
                }
        }
}

func BenchmarkRangeBytes(b *testing.B) {
        bs := make([]byte, 4096)
        for i := 0; i < b.N; i++ {
                for _, _ = range bs {
                }
        }
}


bytes_test.BenchmarkRangeStringLoop  500000  6597 ns/op
bytes_test.BenchmarkRangeStringCast  200000  8083 ns/op
bytes_test.BenchmarkRangeBytes       500000  5020 ns/op

A per-byte (not per-rune) range over a string should be equivalent to a per-byte range
over a []byte, but currently it's 60% slower than it could be.
@bradfitz
Copy link
Contributor Author

Comment 1:

Related, mentioned by Gustavo Niemeyer here:
https://groups.google.com/group/golang-dev/msg/bc0e89aadc748577
`
More seriously, why doing 
    if strings.EqualsBytes(s, b) { ... } 
rather than 
    if string(b) == s { ... } 
If this is just about saving the conversion, should this be done by 
the compiler? 
`

@rsc
Copy link
Contributor

rsc commented Sep 15, 2011

Comment 2:

I am very reluctant to make optimizations like this.
It makes it very hard to tell when something is expensive or not.
If we did the suggested optimization, then 
    for _, _ := range []byte(s) {
would be cheap, while
    b := []byte(s)
    for _, _ := range b {
would be expensive.  Or maybe it could be cheap too, but
    b := []byte(s)
    for i, _ := range b {
        b[i] = 1
    }
would be expensive.  You comment out a line and boom,
expense happens.
I don't see a way around this, and we are probably going
to get forced to make optimizations like this.
(Escape analysis is one.)
But I am still a bit uncomfortable.

Owner changed to @rsc.

Status changed to Accepted.

@lvdlvd
Copy link

lvdlvd commented Sep 15, 2011

Comment 3:

what if we had really good diagnostics, intended to be readable for anyone
that can, say, use a profiler?

@niemeyer
Copy link
Contributor

niemeyer commented Oct 8, 2011

Comment 4:

Another common case:
  m, err := f.Write([]byte("some data"))

@edsrzf
Copy link

edsrzf commented Oct 8, 2011

Comment 5:

In general that can't be optimized. Unless the compiler can deduce the io.Writer's
concrete type, there's no way to know that the Write call doesn't keep an alias to the
[]byte.

@niemeyer
Copy link
Contributor

niemeyer commented Oct 9, 2011

Comment 6:

It can certainly be optimized, even though it's indeed more complex than local analysis.
I didn't say anything about io.Writer, though.

@edsrzf
Copy link

edsrzf commented Oct 9, 2011

Comment 7:

My impression was that the variable f in your example was an io.Writer. Sorry for the
misunderstanding.

@rsc
Copy link
Contributor

rsc commented Dec 9, 2011

Comment 8:

Labels changed: added priority-later.

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 10:

Labels changed: added go1.1maybe.

@robpike
Copy link
Contributor

robpike commented Mar 7, 2013

Comment 11:

Labels changed: removed go1.1maybe.

@rsc
Copy link
Contributor

rsc commented Mar 12, 2013

Comment 12:

[The time for maybe has passed.]

@bradfitz
Copy link
Contributor Author

Comment 13:

Labels changed: added garbage.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 14:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 15:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 16:

Labels changed: added repo-main.

@dvyukov dvyukov closed this as completed in 71be013 Feb 4, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
@rsc rsc removed their assignment Jun 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge GarbageCollector
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@bradfitz @rsc @edsrzf @niemeyer @robpike @gopherbot @lvdlvd