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

Build time regression from the Compound type #239

Open
kvark opened this issue May 25, 2020 · 5 comments
Open

Build time regression from the Compound type #239

kvark opened this issue May 25, 2020 · 5 comments
Labels
bug easy An issue that's easy to implement good first issue Perfect for new contributors help wanted

Comments

@kvark
Copy link
Collaborator

kvark commented May 25, 2020

... introduced in #206
According to #206 (comment), results in 13% increase in IR size.
We want multiple parts of Gecko to ship (at least on Nightly) with RON built-in and enabled, so code size is important: we want that path to be as light as possible.
cc @nnethercote @Plecra

@kvark kvark added the bug label May 25, 2020
@Plecra
Copy link
Contributor

Plecra commented May 26, 2020

I hope we can start with assessing the actual impact - the number of instantiations haven't changed, and LLVM just seems to be reporting 3x the IR size in each function. This doesn't directly reflect the final code size or the compile time (which I'm assuming is what we really care about)

@nnethercote
Copy link
Contributor

LLVM IR size has proven to be a good proxy for compile times, particularly for debug builds. See rust-lang/rust#72013 for one example. But it would certainly be worthwhile for you to check that this observation holds for this case as well.

@nnethercote
Copy link
Contributor

This is the likely cause of an installer size increase for Firefox when Firefox updated to 0.6.2.

@kvark
Copy link
Collaborator Author

kvark commented Sep 17, 2020

@nnethercote looks like bincode-1.3 was able to address it on their side, but I'm not able to find the exact change/PR on a cursory look.

@juntyr
Copy link
Member

juntyr commented Jan 2, 2022

During deserialization we read in and buffer all bytes. Could we do something similar and revert to appending to a String in the Deserializer, which is only flushed to a writer at the end? This would reduce most of the duplication at the cost of not supporting extremely large RON documents.

@juntyr juntyr added help wanted easy An issue that's easy to implement good first issue Perfect for new contributors labels Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug easy An issue that's easy to implement good first issue Perfect for new contributors help wanted
Projects
None yet
Development

No branches or pull requests

4 participants