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

[2/n] - Remove code-size-costly optimizations #507

Merged
merged 3 commits into from
Jun 17, 2021

Conversation

Dirbaio
Copy link
Contributor

@Dirbaio Dirbaio commented Jun 13, 2021

Part 2 of N of #492. Depends on #505

  • Remove bool compression
  • Remove LEB128 compression. usize/isize are now 4 bytes, format tags are now 2 bytes.

Code size comparsion (only including gains from this PR, not #505):

                             before    after    change
debug:                       107232   101452    -5.3% 
release:                      40852    37396    -8.4%
release + flags:              21936    19416   -11.4%
release + flags + buildstd:   20524    18004   -12.3%

rustc 1.54.0-nightly (676ee1472 2021-05-06)
"flags" means these in Cargo.toml:
    codegen-units = 1
    debug = 2
    debug-assertions = false
    incremental = false
    lto = 'fat'
    opt-level = 'z'
    overflow-checks = false

"buildstd" means these in .cargo/config.toml:
    [unstable]
    build-std = ["core"]
    build-std-features = ["panic_immediate_abort"]

- Panics on reentrant acquire.
- Remove `Write` trait.
@Dirbaio Dirbaio force-pushed the remove-optimizations branch 2 times, most recently from 21aff44 to af87dd1 Compare June 13, 2021 22:41
@Dirbaio Dirbaio marked this pull request as ready for review June 13, 2021 22:56
@Dirbaio Dirbaio changed the title Remove code-size-costly optimizations [2/n] Remove code-size-costly optimizations Jun 13, 2021
@Dirbaio Dirbaio changed the title [2/n] Remove code-size-costly optimizations [2/n] - Remove code-size-costly optimizations Jun 13, 2021
@Dirbaio Dirbaio mentioned this pull request Jun 14, 2021
Copy link
Contributor

@jonas-schievink jonas-schievink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

decoder/src/decoder.rs Outdated Show resolved Hide resolved
@Dirbaio Dirbaio force-pushed the remove-optimizations branch from af87dd1 to 6aa2be1 Compare June 16, 2021 14:54
@Dirbaio Dirbaio force-pushed the remove-optimizations branch from 6aa2be1 to 2b87d4b Compare June 16, 2021 15:06
@Urhengulas Urhengulas added breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version status: blocked Blocked on another issue, or on upstream libraries labels Jun 16, 2021
@Urhengulas Urhengulas removed the status: blocked Blocked on another issue, or on upstream libraries label Jun 17, 2021
@Urhengulas
Copy link
Member

The same applies for this PR, as for #505 :)

@jonas-schievink
Copy link
Contributor

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 17, 2021

Build succeeded:

@bors bors bot merged commit ecf25b4 into knurling-rs:main Jun 17, 2021
bors bot added a commit that referenced this pull request Jun 30, 2021
508: [5/n] Format trait v2 r=japaric a=Dirbaio

Part of #492 . Depends on ~~#505 #507 #521~~ #524

- Implemented new Format trait according to #492.
- Adjusted everything else accordingly.

Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants