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

syntax: Join consecutive string literals in format strings together #15832

Closed
wants to merge 1 commit into from
Closed

syntax: Join consecutive string literals in format strings together #15832

wants to merge 1 commit into from

Conversation

bluss
Copy link
Member

@bluss bluss commented Jul 20, 2014

Emit a single rt::Piece for consecutive string literals. String literal chunks
are split on {{ or }} escapes in parsing, and here we join them when expanding.

Saves a small amount of static storage and emitted code size.

Emit a single rt::Piece per consecutive string literals. String literals
are split on {{ or }} escapes.

Saves a small amount of static storage and emitted code size.
@bluss
Copy link
Member Author

bluss commented Jul 20, 2014

starting very simple, re: Show bloat #15548

an rt::Piece is 72 bytes, so this saves 144 bytes on the stack in every derived Show impl for structs, mostly zeros, by making sure "Structname { member: " and " }" are just two String pieces instead of four.

Does this change pay for itself in "performance" increase vs complexity added? cc @alexcrichton

@alexcrichton
Copy link
Member

Do you have any measurements for the improvements gained here? I don't think that this would save space on the stack because all directives are stored in to rodata, no ton the stack. I would, however, expect this to decrease the size of executables/libraries by at least a bit.

This implementation looks good to me, but I'd like to see a measurement of a before/after just to see what we're gaining.

@bluss
Copy link
Member Author

bluss commented Jul 20, 2014

binary size -- I think the change is very very small

 $ size --format=Berkeley new/lib*
   text    data     bss     dec     hex filename
  16704   43227       4   59935    ea1f new/libarena-4e7c5e5c.so
  66353   68691       4  135048   20f88 new/libdebug-4e7c5e5c.so
  31623    3694       8   35325    89fd new/libflate-4e7c5e5c.so
  20497   43949       8   64454    fbc6 new/libfmt_macros-4e7c5e5c.so
  76969   79267       4  156240   26250 new/libgetopts-4e7c5e5c.so
   9800   44700       4   54504    d4e8 new/libgraphviz-4e7c5e5c.so
  30301   38304     200   68805   10cc5 new/liblog-4e7c5e5c.so
 345535  159001      24  504560   7b2f0 new/libnative-4e7c5e5c.so
 165524  177332       4  342860   53b4c new/libregex-4e7c5e5c.so
12815808        5999302     368 18815478        11f19f6 new/librustc-4e7c5e5c.so
 224381  100857       4  325242   4f67a new/librustc_back-4e7c5e5c.so
19020330        1043863   69088 20133281        13335a1 new/librustc_llvm-4e7c5e5c.so
2987929 1069179       4 4057112  3de818 new/librustdoc-4e7c5e5c.so
1161899  487994    5280 1655173  194185 new/librustrt-4e7c5e5c.so
 267564  851740       4 1119308  11144c new/libserialize-4e7c5e5c.so
 833044 1551820     344 2385208  246538 new/libstd-4e7c5e5c.so
 159305  630662       8  789975   c0dd7 new/libsync-4e7c5e5c.so
3216767 3550444       4 6767215  67426f new/libsyntax-4e7c5e5c.so
 212138  102246       8  314392   4cc18 new/libterm-4e7c5e5c.so
 446332  261222       8  707562   acbea new/libtest-4e7c5e5c.so
  56115   61988       4  118107   1cd5b new/libtime-4e7c5e5c.so
$ size --format=Berkeley old/lib*
   text    data     bss     dec     hex filename
  16704   43231       8   59943    ea27 old/libarena-4e7c5e5c.so
  66353   68688       8  135049   20f89 old/libdebug-4e7c5e5c.so
  31563    3694       8   35265    89c1 old/libflate-4e7c5e5c.so
  20501   43950       8   64459    fbcb old/libfmt_macros-4e7c5e5c.so
  76981   79267       4  156252   2625c old/libgetopts-4e7c5e5c.so
   9820   44702       8   54530    d502 old/libgraphviz-4e7c5e5c.so
  30357   38835     200   69392   10f10 old/liblog-4e7c5e5c.so
 345551  158983      24  504558   7b2ee old/libnative-4e7c5e5c.so
 165488  177654       8  343150   53c6e old/libregex-4e7c5e5c.so
12817208        6004915     368 18822491        11f355b old/librustc-4e7c5e5c.so
 224345  100862       8  325215   4f65f old/librustc_back-4e7c5e5c.so
19020330        1043869   69088 20133287        13335a7 old/librustc_llvm-4e7c5e5c.so
2987941 1071291       4 4059236  3df064 old/librustdoc-4e7c5e5c.so
1162110  488450    5280 1655840  194420 old/librustrt-4e7c5e5c.so
 267596  852193       4 1119793  111631 old/libserialize-4e7c5e5c.so
 833128 1552192     344 2385664  246700 old/libstd-4e7c5e5c.so
 159261  630741       8  790010   c0dfa old/libsync-4e7c5e5c.so
3220819 3562525       8 6783352  678178 old/libsyntax-4e7c5e5c.so
 212270  102337       4  314611   4ccf3 old/libterm-4e7c5e5c.so
 446524  261683       4  708211   ace73 old/libtest-4e7c5e5c.so
  56171   62442       4  118617   1cf59 old/libtime-4e7c5e5c.so

@alexcrichton
Copy link
Member

Looks like it's still a win to me though!

bors added a commit that referenced this pull request Sep 9, 2014
Based on an observation that strings and arguments are always interleaved, thanks to #15832. Additionally optimize invocations where formatting parameters are unspecified for all arguments, e.g. `"{} {:?} {:x}"`, by emptying the `__STATIC_FMTARGS` array. Next, `Arguments::new` replaces an empty slice with `None` so that passing empty `__STATIC_FMTARGS` generates slightly less machine code when `Arguments::new` is inlined. Furthermore, formatting itself treats these cases separately without making redundant copies of formatting parameters.

All in all, this adds a single mov instruction per `write!` in most cases. That's why code size has increased.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 13, 2023
…cola

feat: add generate_mut_trait_impl assist

![generate_mut_trait_impl](https://github.com/rust-lang/rust-analyzer/assets/71162630/362a5a93-e109-4ffc-996e-9b6e4f54fcfa)

Generate proper `index_mut` method body refer to `index` method body may impossible due to the unpredicable case (rust-lang#15581).

Here just leave the `index_mut` method body be same as `index` method body, user can modify it manually to meet their need.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants