-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
libcore: Make fail handlers optional #18277
Conversation
Could you expand a little more on how you see this working? I thought libcore wouldn't compile without this fake module? |
d5ced92
to
baa1f2e
Compare
Some users have no need for formatted fail messages and really can't bear the code sizes that they require. See rust-lang#17886.
baa1f2e
to
468e2cd
Compare
Whoops, yes, you are right. My flag apparently wasn't making it through the Zinc build system to I just pushed another attempt which this time I confirmed was built with |
In the PR message you explained the motivation for this as "some users would rather define their own fail handlers," but as written this hard-codes all failure to |
@alexcrichton it wasn't intentional but it happened ;) I neglected to update the PR text. It has since been fixed. That being said, it is a bit unfortunate that we now lose the ability to define custom fail handlers. I added a patch on top to instead just eliminate the arguments of At this point I'm weakly in support of this second approach as I think we'll really want the ability to override |
Interesting! With the goal of getting as small as possible here, I'm surprised that only removing the formatting messages from these failure functions is sufficient to fit your constraints. When you say 1096 bytes to 916 bytes, is that a before/after with respect to the entire patch or just the first/second strategy? I'm curious about what's taking up the most space here which you you want to eliminate. The factors which contribute to the size of the code that I know of are:
Of these, the current PR only eliminates 2/3 for the two functions in I suppose I could summarize my question as follows: Do you have some more data about the before/after picture here with respect to both the approaches? (forbidding overriding or just removing msgs) Also, as a side note, while you're modifying these could you change the |
@alexcrichton here is a more thorough tabulation (looking at zinc with
|
Wow, I would most certainly not expect the current patch to be under half the size of the original one. Could you past the before/after IR? I'm curious as to what's causing all the extra |
I don't have IR at hand, but I remember that bloat is introduced by |
@alexcrichton I haven't yet generated IR but looking at
By my tally those constitute 1180 bytes of text. These also appear,
Which is another 187 bytes for a total of 1367 bytes. |
The plot thickens! So to explain my impetus for these questions, I do find it unfortunate that we have a mode for libcore where failure produces blank failure messages where it otherwise has useful contextual information about what happened. The size does seem worrying, however, and I'd love to optimize it! It looks like the largest contributor is the
Would it be possible to investigate optimizing the size of the padding functions before adding a switch to turn off the error messages entirely? One possible route to take is to define The more difficult route (but more general) would be optimizing the padding function itself. The first step here would probably be removing as many points of "failure" as possible, with the next being optimizing the amount of code. This would apply more generally and would help keep all binaries a tad bit smaller perhaps? How does that sound? |
@alexcrichton, one of the things is that the system might run completely "headless" (no way to return any string of text to the user), or spend most of the boot time headless. Having any dependencies to pull in text strings and formatting is still dead weight, and in some cases each byte counts. |
Taking that to the extreme then the |
Closing due to inactivity (and @bors's queue is quite full!). Feel free to reopen with rebase! |
Alex Crichton notifications@github.com writes:
|
Some users, particularly those targeting embedded systems, would rather avoid depending on
fmt
due to its effect on code size. See #17886.