You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I think that instead of calling s.next() on line 809 we should both record the current char and call s.next() immediately before the entire nested switch statement. That would remove the double call to isLetter and also remove the need for the s.peek() on line 805. What do you think? It's very possible I'm overlooking something. Let me know if I should put together a PR.
The text was updated successfully, but these errors were encountered:
There are benchmarks. Does any change you can come up with actually improve the benchmarks with benchstat? If not, then it's probably not worth optimizing.
danaugrs
changed the title
Potential scanner inefficiency
go/scanner: potential inefficiency
Nov 14, 2019
Before modifying and benchmarking something I find it beneficial to discuss it with those familiar with the matter. Is anyone familiar with the scanner logic interested in reasoning about the inefficiency I described and my proposal? @griesemer
It is almost always better to write code that is clear and easy to adjust in the future than code that is awkward but saves a few cycles. In this case, the string conversion far outweighs the cost of potentially looking up the first character twice. Even if the string conversion were not here, the current code can't be running slower than if every identifier were just one character longer, which we know doesn't really affect time. (Otherwise everyone would be saying to use short names in your program to make it parse faster.)
There's no change to make here, but thanks for taking the time to file the issue.
I noticed a potential scanner inefficiency here:
go/src/go/scanner/scanner.go
Lines 791 to 793 in 3f21c23
s.scanIdentifier()
callsisLetter(s.ch)
with the same char that was just checked in the case condition:go/src/go/scanner/scanner.go
Lines 350 to 356 in 3f21c23
I think that instead of calling
s.next()
on line 809 we should both record the current char and calls.next()
immediately before the entire nested switch statement. That would remove the double call toisLetter
and also remove the need for thes.peek()
on line 805. What do you think? It's very possible I'm overlooking something. Let me know if I should put together a PR.The text was updated successfully, but these errors were encountered: