Skip to content

cmd/compile: readonly analysis of functions accepting []byte arguments #29810

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.

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

Closed
josharian opened this issue Jan 18, 2019 · 4 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone

Comments

@josharian
Copy link
Contributor

In #29802, @rillig observes that encoding/hex.Decode only reads from its []byte argument. (In particular, it calls len and reads bytes at particular indices.) We could teach the compiler that for such arguments, we can avoid an alloc+copy when passing an argument converted from a string, the way we do with map lookups.

The obvious question is how often this occurs, and whether it justifies the cost (implementation, maintenance, code complexity, execution time).

I thought that we already had an issue for this, but I can't find it.

cc @martisch @mvdan @randall77

@josharian josharian added Performance NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 18, 2019
@josharian josharian added this to the Go1.13 milestone Jan 18, 2019
@pwaller
Copy link
Contributor

pwaller commented Jan 18, 2019

Possible dupe of #2205 ?

@mvdan
Copy link
Member

mvdan commented Jan 18, 2019

I was trying to find the issue that @pwaller just linked to :)

@martisch
Copy link
Contributor

martisch commented Jan 18, 2019

The use case for maps is I think the other way around where the compiler can eliminate the copy for conversion to string (as opposed to from string). In contrast ranging over []byte(string) the copy is avoided by the compiler. However both require knowledge that the byte slice will not change.

I dont think we need to make the distinction if the argument is passed in as a conversion like for maps. If we have a compiler pass that can evaluate if a []byte slice is ever written to within a specific range even in functions its passed to we can generally avoid the copy on conversion from/to string (even if its stand alone before any call e.g. map operation) and also use it to prove that e.g. for x := range string(a) if a is never modified in the for loop the copy can be avoided #27148.

A problem might be that walk pass seems not optimal for this kind of analysis and that after ssa/call construction it might be too low level already. If we had a high level ssa structure with calls as high level concepts to optimize on, some new and existing optimizations maybe easier to implement and maintain. I dont remember the issue but this IIRC this was discussed before. (Maybe I was thinking of #17728)

@josharian
Copy link
Contributor Author

Possible dupe of #2205 ?

Yes, thanks. :) I knew there was one. I'll close this one.

A problem might be that walk pass seems not optimal for this kind of analysis and that after ssa/call construction it might be too low level already.

Indeed. Although this might be a common enough case, with a simple enough analysis, that it's worth doing even the lower-power version available during walk. Might be worth prototyping.

If we had a high level ssa structure with calls as high level concepts to optimize on, some new and existing optimizations maybe easier to implement and maintain. I dont remember the issue but this IIRC this was discussed before.

Yes, this ordering issue is a perennial problem. It has also showed up in discussions of inlining, purity analysis, DCE, and more.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Development

No branches or pull requests

5 participants