-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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
strconv: optimize Parse for []byte arguments #42429
Comments
At my company we have shared functions that do this too. They see a reasonable amount of use in high-performance code. It's not hard to write the unsafe code, but it's not completely trivial either -- it's easy to overlook the problem of the string being retained unsafely inside the |
Don't forget option (3): wait for a decision on generics, and then provide an updated |
If there's a new strconv library that's generic over |
If something like #2205 (comment) was implemented, perhaps we could get the compiler to do the work for us after all. That said, I'm not in favor of simply waiting another few years for that to happen. |
Similar to option (3): byte view that can represent |
If there was a compiler optimisation for this, it would be good if it could be generalised to support |
Maybe, it is not a too bad idea to let slices comparable. Comparing slices is just like comparing strings. |
Sorry, I really misunderstood @rogpeppe's last comment. In my case, I often want structs with byte slice fields are hashable and can be used as map keys but I have to convert the slices to strings in other structs to get hashable values. [update]: I have ever made a proposal to solve the problem mentioned in rogpeppe's last comment: https://github.com/go101/go101/wiki/A-proposal-to-avoid-duplicating-underlying-bytes-when-using-strings-as-read-only-%5B%5Dbyte-arguments |
zeebo in Gopher Slack made a patch that eliminates most of the allocations: https://gist.github.com/zeebo/aeb001d35ca3a6e0ebd85eb52e3ef4c2 |
Change https://golang.org/cl/345488 mentions this issue: |
In 2016, #2632 was closed with the explanation that:
In summary, it's argued that 1) the compiler should address this, and 2) people can vendor and modify the
strconv
package.It's been 4 years since the issue was closed (and 9 years since the issue was first reported), and there there hasn't been a compiler optimization that permits calling the
Parse
functions with a[]byte
without it creating garbage. That is:always (as of Go1.15) allocates and copies the input
[]byte
.Forking
strconv
is an unfavorable workaround because it means that the vendor fails to benefit from future optimizations tostrconv
(e.g., cl/187957 or cl/260858) and fixes to the implementation (e.g., #29491) and may go against company guidelines that forbid forks. Personally, I haven't seen anyone forkstrconv
. Rather, in most cases I see users cast a[]byte
to astring
usingunsafe
. This is what we do in the protobuf module. However, some use-cases are constrained to avoid usingunsafe
(e.g.,encoding/json
) and so continue to suffer from performance losses.Given the passage of time and the lack of a natural solution to this, I think we should revisit this issue. It seems that we should either:
[]byte
tostring
in the example snippet above. If such a compiler optimization existed, we would probably also need to modifystrconv
to avoid escaping the string input through the error value by copying the input string. Note that this will slow down cases where parsing fails.ParseBoolBytes
,ParseComplexBytes
,ParseFloatBytes
,ParseIntBytes
, andParseUintBytes
, which are identical to their counterparts without theBytes
suffix, but operate on a[]byte
as the input instead of astring
. (We can addBytes
equivalents for theQuoteXXX
functions as well, but those seem to be rarely with a[]byte
.)Obviously option 1 is preferred, but if its not going to happen, but perhaps it's time to do option 2. It's been almost a decade.
\cc @mvdan @rogpeppe @mdlayher
The text was updated successfully, but these errors were encountered: