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

Add #[inline] to allow for cross-crate inlining #33

Merged
merged 1 commit into from
Jul 6, 2015

Conversation

cesarb
Copy link
Contributor

@cesarb cesarb commented Jul 5, 2015

Without #[inline], the function definition is not exported for inlining by other crates. For the byte order conversion functions, inlining can make a great difference in code quality.

For example, trying this small test crate:

extern crate byteorder;

use byteorder::{ByteOrder, LittleEndian};

pub fn example(buf: &[u8; 8]) -> u64 {
    LittleEndian::read_u64(buf)
}

The result without #[inline] in the byteorder crate:

_ZN7example20h618e367dcb99bd17iaaE:
        .cfi_startproc
        cmpq    %fs:112, %rsp
        ja      .LBB0_2
        movabsq $24, %r10
        movabsq $0, %r11
        callq   __morestack
        retq
.LBB0_2:
        subq    $24, %rsp
.Ltmp0:
        .cfi_def_cfa_offset 32
        movq    %rdi, 8(%rsp)
        movq    $8, 16(%rsp)
        leaq    8(%rsp), %rdi
        callq   _ZN22LittleEndian.ByteOrder8read_u6420hc703c516084f60e83EaE@PLT
        addq    $24, %rsp
        retq

The result after adding #[inline] to the byteorder crate:

_ZN7example20h52794dd36d3f283eiaaE:
        .cfi_startproc
        movq    (%rdi), %rax
        retq

As you can see, LittleEndian::read_u64(...) on a little-endian processor, when the compiler can determine statically that the buffer is large enough, can be inlined into a single "load" instruction.

The same should apply to all the functions I marked as #[inline] in this pull request.

BurntSushi added a commit that referenced this pull request Jul 6, 2015
Add #[inline] to allow for cross-crate inlining
@BurntSushi BurntSushi merged commit 78d304f into BurntSushi:master Jul 6, 2015
@BurntSushi
Copy link
Owner

Superb. I buy it. Thanks!

@BurntSushi
Copy link
Owner

I've included this fix in 0.3.11 on crates.io. Thanks again!

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