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 initial support for a new formatting syntax #8245

Closed
wants to merge 1 commit into from

Conversation

alexcrichton
Copy link
Member

This is a reopening of #8182, although this removes any abuse of the compiler internals. Now it's just a pure syntax extension (hard coded what the attribute names are).

@@ -0,0 +1,901 @@
// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: what does ct mean in the file name? Maybe change to something more descriptive.

@pcwalton
Copy link
Contributor

pcwalton commented Aug 3, 2013

From looking at this it looks like the format string verified at compile time but parsed at runtime. That seems strange: is it due to internalization concerns? I would hope that for perf there is at least eventually an option to turn off runtime parsing.

@alexcrichton
Copy link
Member Author

Good news! For the following benchmark:

extern mod extra;
use extra::test::BenchHarness;

#[bench]
fn fmt(b: &mut BenchHarness) {
    do b.iter {
        do 100.times {
            fmt!("aasd f df %d awe", 4);
        }
    }
}

#[bench]
fn ifmt(b: &mut BenchHarness) {
    do b.iter {
        do 100.times {
            ifmt!("aasd f df {:d} awe", 4);
        }
    }
}

I get the results:

running 2 tests
test fmt ... bench: 31349 ns/iter (+/- 1084)
test ifmt ... bench: 20443 ns/iter (+/- 403)

test result: ok. 0 passed; 0 failed; 0 ignored; 2 measured

Here's a summary:

  • All format strings are now compiled and generate lots and lots and lots of statics all over the place. These statics are then passed into sprintf which doesn't have to parse anything at that point
  • It now uses a &mut io::Writer instead of string implicitly.
  • I modified rt::io::MemWriter to allocate a vector with 128 bytes reserved. That sped up ifmt! performance by a lot, but I may have also been confusing this with other improvements I was getting
  • I'm trying to keep everything on "the fast path" as much as possible. Currently sprintf only makes one allocation, and it's because you're returning an owned string anyway.
  • Changed ct to parse

On the side I also change:

  • Used a condition instead of error function since my fix landed
  • All named arguments are no longer passed as named arguments. All references to them are rewritten to be a positional argument at the end of the list of arguments.
  • All references to the implicit "next argument" are disallowed in nested select/plural because it wouldn't make sense if different arms used different amounts of "next arguments"

I'm still fixing up a few bugs here and there, but this has the bulk of everything.

@alexcrichton
Copy link
Member Author

All bugs should be fixed now, everything should be good to go

@graydon graydon mentioned this pull request Aug 7, 2013
@alexcrichton
Copy link
Member Author

Here's some numbers about code size generated from the new formatting syntax. This program was compiled

#[inline(never)]
#[no_mangle]
fn fmtarg0() -> ~str {
    fmt!("Hello world for the 4th time again!")
}

#[inline(never)]
#[no_mangle]
fn ifmtarg0() -> ~str {
    ifmt!("Hello world for the 4th time again!")
}

#[inline(never)]
#[no_mangle]
fn fmtarg1() -> ~str {
    fmt!("Hello %s for the 4th time again!", "world")
}

#[inline(never)]
#[no_mangle]
fn ifmtarg1() -> ~str {
    ifmt!("Hello {:s} for the 4th time again!", "world")
}

#[inline(never)]
#[no_mangle]
fn fmtarg2() -> ~str {
    fmt!("Hello %s for the %dth time again!", "world", 4)
}

#[inline(never)]
#[no_mangle]
fn ifmtarg2() -> ~str {
    ifmt!("Hello {:s} for the {:d}th time again!", "world", 4)
}

#[inline(never)]
#[no_mangle]
fn fmtarg3() -> ~str {
    fmt!("Hello %s for the %dth time %s!", "world", 4, "again")
}

#[inline(never)]
#[no_mangle]
fn ifmtarg3() -> ~str {
    ifmt!("Hello {:s} for the {:d}th time {:s}!", "world", 4, "again")
}

fn main() {
}

And I got this data:

args    fmt     ifmt
0       54      98  + 203 = 301
1       207     136 + 262 = 398
2       310     163 + 320 = 483
3       440     200 + 379 = 579

The first number for ifmt is the code size of the function, and the second number is the size of the static symbol generated for the format string (or at least I think it's the right size). Basically ifmt is almost always smaller code at the callsite, but overall a larger code impact. To optimize this it's probably mostly just struct layout in general. I could try to optimize this if we think it's a problem.

@emberian
Copy link
Member

emberian commented Aug 8, 2013

Closing; in my rollup

@emberian emberian closed this Aug 8, 2013
@emberian
Copy link
Member

emberian commented Aug 8, 2013

Backing out of my rollup

@emberian emberian reopened this Aug 8, 2013
The new macro is available under the name ifmt! (only an intermediate name)
bors added a commit that referenced this pull request Aug 8, 2013
This is a reopening of #8182, although this removes any abuse of the compiler internals. Now it's just a pure syntax extension (hard coded what the attribute names are).
@bors bors closed this Aug 8, 2013
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.

5 participants