Implement embedded FS directly with strings #376
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In profiling #275, I discovered that a good chunk of time and memory allocation is due to the embedded FS. Our VFS interface reads file contents in terms of strings, but the underlying
io/fs.FS
interface uses mutable byte slices, so that incurs a copy for each file read. If you keep going down, it turns out thatgo:embed
is implemented in terms of immutable in-binary strings, which are then copied out of byte slices! So, each time we access the embedded VFS, we are converting a string to a byte slice, just to turn it back into a string.If we skip the
io/fs.FS
layer and instead directly implement our ownvfs.FS
interface in terms ofgo:embed
'd strings, we can skip this entirely.On my machine, this brings the total runtime of
internal/testutil/runner
from 9.3s to 6.3s, reducing memory allocation during the test from 42.7GB to 10.25GB.I'm not 100% happy with the implementation itself (mainly if we choose to add more stuff, though I doubt I'd do the translations as a FS layer), but, I think it's okay for now.