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.

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

x/text/unicode/norm: reduce init-time memory usage #26752

Open
bradfitz opened this issue Aug 1, 2018 · 7 comments
Open

x/text/unicode/norm: reduce init-time memory usage #26752

bradfitz opened this issue Aug 1, 2018 · 7 comments
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone
Unplanned

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Aug 1, 2018 •
edited

The golang.org/x/text/unicode/norm package allocs 21.47kB in its init function, regardless of whether the package is ever used.

We should do it lazily, as done in e.g. https://go-review.googlesource.com/c/net/+/127275 and https://go-review.googlesource.com/c/go/+/127075

This would help people who explicitly import it (or import it via golang.org/x/net/http2), as then the std-vendored copy would not need to also allocate the 20KB.

Perhaps some of those tables should be const strings, too, so the linker can put it in read-only sections. Then we can slice into it at runtime to get integers out.

/cc @mpvl
@bradfitz bradfitz added Performance help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Aug 1, 2018
@bradfitz bradfitz added this to the Unplanned milestone Aug 1, 2018
@ianlancetaylor
Copy link
Contributor

CC @mpvl

@bradfitz bradfitz mentioned this issue Aug 2, 2018
Open
@kevinburke
Copy link
Contributor

Going to take a crack at this!

@gopherbot
Copy link

Change https://golang.org/cl/127924 mentions this issue: unicode/norm: lazy load decomps table

@gopherbot
Copy link

Change https://golang.org/cl/127926 mentions this issue: make recompMap a string

gopherbot pushed a commit to golang/text that referenced this issue Aug 7, 2018
@kevinburke @bradfitz
unicode/norm: lazy load recompMap from string
2378460 
recompMap gets populated on init, which requires 21kB of memory.
Instead populate it only when we need it.

This will put it in the rodata section of the binary, which means that
the Go compiler can apply better optimizations to it. The string is an
alternating series of 4 byte keys and 4 byte values, which get built
into a map the first time they are required.

Updates golang/go#26752.

Change-Id: I4f9b7d74af6df42b58999cf42cf5ada9646aa98d
Reviewed-on: https://go-review.googlesource.com/127926
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@agnivade
Copy link
Contributor

Is there a way to generate the tables for all unicode versions ? If I run "go generate", it only generates the tables for the current Go version.

Also, how are you checking the memory allocation of the init function ? If I just do a blank import and run a mem profile, nothing gets caught.

@bradfitz
Copy link
Contributor Author

@agnivade, see the top comment on #26775 and https://twitter.com/bradfitz/status/1024726819725950976

@gopherbot
Copy link

Change https://golang.org/cl/207017 mentions this issue: unicode/norm: make decomps to a const string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@bradfitz @kevinburke @agnivade @ianlancetaylor @gopherbot