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

Turn FluentBundle to be a generic over Borrow<FluentResource> #114

Merged
merged 1 commit into from
Jul 10, 2019

Conversation

zbraniecki
Copy link
Collaborator

Fixes #113

@zbraniecki zbraniecki force-pushed the res-generic branch 2 times, most recently from cd60c01 to 0dcd0db Compare July 9, 2019 21:35
@zbraniecki zbraniecki requested a review from stasm July 9, 2019 21:36
@zbraniecki
Copy link
Collaborator Author

The patch is complete. This drastically improves the ergonomics of working with FluentBundle.

One can now do the "simple" thing of pushing Resource to Bundle, or store it in some manager and pass references, or store Arc/Rc and pass them.

This is necessary for https://bugzilla.mozilla.org/show_bug.cgi?id=1560038 to allow me to hook into refcounted WebIDL objects.

I kept the indices, because I don't see a cheap way to avoid them. I understand that they're not pretty, but they're very local and in the current model there is no way to mess with them since FluentResource is immutable after construction and we don't support any other operations except of add_resource at the moment. I hope it's ok.

@zbraniecki
Copy link
Collaborator Author

perf:

     Running /Users/zbraniecki/projects/fluent/fluent-rs/target/release/deps/resolver-84e6f1639e28e79a
Gnuplot not found, disabling plotting
resolve/"simple"        time:   [8.9838 us 9.0265 us 9.0719 us]                             
                        change: [+0.0020% +0.0887% +2.0119%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 10 outliers among 100 measurements (10.00%)
  5 (5.00%) high mild
  5 (5.00%) high severe
resolve/"preferences"   time:   [132.89 us 134.58 us 136.97 us]                                  
                        change: [+0.1132% +1.1087% +2.2917%] (p = 0.04 < 0.05)
                        Change within noise threshold.
Found 10 outliers among 100 measurements (10.00%)
  6 (6.00%) high mild
  4 (4.00%) high severe
resolve/"menubar"       time:   [37.254 us 37.435 us 37.629 us]                               
                        change: [-0.0559% +0.8498% +1.7545%] (p = 0.07 > 0.05)
                        No change in performance detected.
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) high mild
  3 (3.00%) high severe
resolve/"unescape"      time:   [2.0841 us 2.0939 us 2.1048 us]                                
                        change: [+0.0298% +0.7207% +1.4294%] (p = 0.05 < 0.05)
                        Change within noise threshold.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) low mild
  5 (5.00%) high mild

Gnuplot not found, disabling plotting

Copy link
Contributor

@stasm stasm left a comment

Choose a reason for hiding this comment

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

Looks great, nice work!

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.

Switch FluentBundle to store `R: Borrow<FluentResource>
2 participants