suggested optimization: string literal embedding
stapelberg opened this issue · 6 comments
Take a look at dsymonds/goembed#1 — using string literals speeds up compilation massively. I’ll send another PR on goembed soon to reduce the size of the generated string literal.
Using the same techniques as the goembed PRs should do away with the resource blow-up you mention in your README.md.
go.rice already uses string literals and it also uses %q for formatting, which means it only encodes characters that need to be encoded (similar to dsymonds/goembed#3). I've benchmarked both goembed and go.rice, the memory usage and elapsed time for a single, 80 megabyte file are quite similar.
rice embed-go:
Elapsed (wall clock) time (h:mm:ss or m:ss): 0:06.37
Maximum resident set size (kbytes): 1988272
go build with file generated by rice:
Elapsed (wall clock) time (h:mm:ss or m:ss): 0:04.19
Maximum resident set size (kbytes): 570256
goembed:
Elapsed (wall clock) time (h:mm:ss or m:ss): 0:05.72
Maximum resident set size (kbytes): 7272
go build with file generated by goembed:
Elapsed (wall clock) time (h:mm:ss or m:ss): 0:03.47
Maximum resident set size (kbytes): 624816
The only huge difference is the memory required by the rice command.
Did you also measure resident set size of the compiled program?
goembed:
Maximum resident set size (kbytes): 87232
rice:
Maximum resident set size (kbytes): 88312
(This is after serving the file, otherwise it won't be loaded into memory)
The rice command uses that much memory because it reads every file into memory first, then generates the entire source code, then parses that and formats it (so it keeps three copies of the file contents in memory).
So, it looks like some optimizations are possible for the rice command itself.. Probably the first thing would be to make the embed command use streams instead of reading the whole file. I'm quite busy these days, if anyone could pick this up that would be great. It could be a while until I find the time to do so.
-edit: just now reading #92-
Closing this issue in favor of #92