projectfluent / fluent-rs

Rust implementation of Project Fluent
https://projectfluent.org
Apache License 2.0
1.07k stars 96 forks source link

Switch to Write for format_pattern #127

Closed zbraniecki closed 3 years ago

zbraniecki commented 5 years ago

Fixes #121.

zbraniecki commented 5 years ago

The initial benchmarks for a scenario where the String is already allocated are pretty promising:

Zibis-MacBook-Pro:fluent-bundle (master)$ git co bundle-writer
Switched to branch 'bundle-writer'
Zibis-MacBook-Pro:fluent-bundle (bundle-writer)$ cargo bench
   Compiling fluent-bundle v0.6.0 (/Users/zbraniecki/projects/fluent/fluent-rs/fluent-bundle)
    Finished release [optimized] target(s) in 3.31s
     Running /Users/zbraniecki/projects/fluent/fluent-rs/target/release/deps/fluent_bundle-7bfff413cd359b88

running 1 test
test types::tests::value_from_copy_ref ... ignored

test result: ok. 0 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out

     Running /Users/zbraniecki/projects/fluent/fluent-rs/target/release/deps/resolver-90a8ad21eb4c2c9d
Gnuplot not found, disabling plotting
resolve/"simple"        time:   [5.7215 us 5.7628 us 5.8204 us]                              
                        change: [-8.0114% -7.2865% -6.5787%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  2 (2.00%) high mild
  2 (2.00%) high severe
resolve/"preferences"   time:   [59.778 us 60.003 us 60.234 us]                                  
                        change: [-33.028% -32.568% -32.146%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) low mild
  1 (1.00%) high mild
resolve/"menubar"       time:   [20.778 us 20.858 us 20.948 us]                               
                        change: [-10.406% -9.5800% -8.7977%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) low mild
  4 (4.00%) high mild
  1 (1.00%) high severe
resolve/"unescape"      time:   [867.19 ns 873.41 ns 882.86 ns]                                
                        change: [-29.669% -29.159% -28.653%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild
  3 (3.00%) high severe

Gnuplot not found, disabling plotting

The particular scenario where the string is allocated is when we have a String in Gecko that we need to populate from Rust - https://bugzilla.mozilla.org/show_bug.cgi?id=1560038

The concern here is that the size of resolve.rs basically doubles because I need to keep two variants of every methods - one for resolution (which wants to carry FluentValues around and compare them) and one for formatting (which writes to writer immediately). The only reason we keep the resolve is for selector resolution, so maybe there's some room to simplify it there? Alternatively, maybe we could macro it out and have the macro produce two versions - one for writer, one for resolver?

zbraniecki commented 3 years ago

superseeded by https://github.com/projectfluent/fluent-rs/pull/199