Concurrent types

[zbraniecki]

This patchset separates out IntlMemoizer and concurrent::IntlMemoizer and then FluentBundle and concurrent::FluentBundle.

This allows Gecko to use FluentBundle without Mutex (using RefCell instead), and all uses of Send + Sync will use concurrent::FluentBundle.

[zbraniecki]

@emilio - can you take a look? Manish helped me design this solution that will hopefully address the concern you raised wrt. Mutex.

I kept Mutex in the concurrent version, because it doesn't seem like we yet have an easy way to use the parking_lot's upgrade functionality here, so hope that Mutex is fine, esp since we won't use that in Gecko.

[Manishearth]

I think in the current design there might be a way to use RWLock with upgrading, but I'd land this first and then experiment with that

[zbraniecki]

@Manishearth I tried to update handlebars_fluent to use this branch - https://github.com/zbraniecki/handlebars-fluent/tree/concurrent_fb

The error I'm seeing is:

error[E0277]: `(dyn std::any::Any + 'static)` cannot be sent between threads safely
 --> tests/template.rs:4:1
  |
4 | / simple_loader!(load, "./tests/locales", "en-US", core: "./tests/locales/core.ftl", customizer: |bundle| {
5 | |     bundle.set_use_isolating(false)
6 | | });
  | |___^ `(dyn std::any::Any + 'static)` cannot be sent between threads safely
  |
  = help: the trait `std::marker::Send` is not implemented for `(dyn std::any::Any + 'static)`
  = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<(dyn std::any::Any + 'static)>`
  = note: required because it appears within the type `std::boxed::Box<(dyn std::any::Any + 'static)>`
  = note: required because it appears within the type `(std::any::TypeId, std::boxed::Box<(dyn std::any::Any + 'static)>)`
  = note: required because of the requirements on the impl of `std::marker::Send` for `hashbrown::raw::RawTable<(std::any::TypeId, std::boxed::Box<(dyn std::any::Any + 'static)>)>`
  = note: required because it appears within the type `hashbrown::map::HashMap<std::any::TypeId, std::boxed::Box<(dyn std::any::Any + 'static)>, std::hash::BuildHasherDefault<fxhash::FxHasher>>`
  = note: required because it appears within the type `std::collections::HashMap<std::any::TypeId, std::boxed::Box<(dyn std::any::Any + 'static)>, std::hash::BuildHasherDefault<fxhash::FxHasher>>`
  = note: required because it appears within the type `std::option::Option<std::collections::HashMap<std::any::TypeId, std::boxed::Box<(dyn std::any::Any + 'static)>, std::hash::BuildHasherDefault<fxhash::FxHasher>>>`
  = note: required because it appears within the type `type_map::TypeMap`
  = note: required because it appears within the type `intl_memoizer::IntlLangMemoizer`
  = note: required because of the requirements on the impl of `std::marker::Sync` for `std::sync::Mutex<intl_memoizer::IntlLangMemoizer>`
  = note: required because it appears within the type `fluent_bundle::bundle::FluentBundleBase<&'static fluent_bundle::resource::FluentResource, std::sync::Mutex<intl_memoizer::IntlLangMemoizer>>`
  = note: required because it appears within the type `(unic_langid_impl::LanguageIdentifier, fluent_bundle::bundle::FluentBundleBase<&'static fluent_bundle::resource::FluentResource, std::sync::Mutex<intl_memoizer::IntlLangMemoizer>>)`
  = note: required because of the requirements on the impl of `std::marker::Sync` for `hashbrown::raw::RawTable<(unic_langid_impl::LanguageIdentifier, fluent_bundle::bundle::FluentBundleBase<&'static fluent_bundle::resource::FluentResource, std::sync::Mutex<intl_memoizer::IntlLangMemoizer>>)>`
  = note: required because it appears within the type `hashbrown::map::HashMap<unic_langid_impl::LanguageIdentifier, fluent_bundle::bundle::FluentBundleBase<&'static fluent_bundle::resource::FluentResource, std::sync::Mutex<intl_memoizer::IntlLangMemoizer>>, std::collections::hash_map::RandomState>`
  = note: required because it appears within the type `std::collections::HashMap<unic_langid_impl::LanguageIdentifier, fluent_bundle::bundle::FluentBundleBase<&'static fluent_bundle::resource::FluentResource, std::sync::Mutex<intl_memoizer::IntlLangMemoizer>>>`
  = note: required by `lazy_static::lazy::Lazy`
  = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

which feels like it should be coming from intl_memoizer, but there's no custom Any type in it. Does this error seem meaningful to you?

[Manishearth]

TypeMap has an Any on it, you need to use the concurrent typemap

[Manishearth]

So basically we should have two separate types, Mutex<concurrent::IntlLangMemoizer> and RefCell<IntlLangMemoizer>. This is actually better for using RWlock's upgrade eventually.

[zbraniecki]

--- a/fluent-bundle/src/concurrent.rs
+++ b/fluent-bundle/src/concurrent.rs
@@ -1,6 +1,6 @@
 use std::sync::Mutex;

-use intl_memoizer::{IntlLangMemoizer, Memoizable};
+use intl_memoizer::{concurrent::IntlLangMemoizer, Memoizable};
 use unic_langid::LanguageIdentifier;

 use crate::bundle::FluentBundleBase;

I changed this and now the error is:

▶ cargo check
    Checking fluent-bundle v0.10.2 (/projects/fluent-rs/fluent-bundle)
error[E0277]: `I` cannot be sent between threads safely
  --> fluent-bundle/src/concurrent.rs:30:41
   |
26 |         U: FnOnce(&I) -> R,
   |                            - help: consider further restricting type parameter `I`: `, I: std::marker::Send`
...
30 |                 Ok(memoizable) => Ok(cb(&memoizable)),
   |                                         ^^^^^^^^^^^ `I` cannot be sent between threads safely
   |
   = help: the trait `std::marker::Send` is not implemented for `I`

error[E0277]: `I` cannot be shared between threads safely
  --> fluent-bundle/src/concurrent.rs:30:41
   |
26 |         U: FnOnce(&I) -> R,
   |                            - help: consider further restricting type parameter `I`: `, I: std::marker::Sync`
...
30 |                 Ok(memoizable) => Ok(cb(&memoizable)),
   |                                         ^^^^^^^^^^^ `I` cannot be shared between threads safely
   |
   = help: the trait `std::marker::Sync` is not implemented for `I`

error[E0277]: `<I as intl_memoizer::Memoizable>::Args` cannot be shared between threads safely
  --> fluent-bundle/src/concurrent.rs:29:48
   |
26 |         U: FnOnce(&I) -> R,
   |                            - help: consider further restricting the associated type: `, <I as intl_memoizer::Memoizable>::Args: std::marker::Sync`
...
29 |             Ok(mut memoizer) => match memoizer.try_get(args) {
   |                                                ^^^^^^^ `<I as intl_memoizer::Memoizable>::Args` cannot be shared between threads safely
   |
   = help: the trait `std::marker::Sync` is not implemented for `<I as intl_memoizer::Memoizable>::Args`

error[E0277]: `<I as intl_memoizer::Memoizable>::Args` cannot be sent between threads safely
  --> fluent-bundle/src/concurrent.rs:29:48
   |
26 |         U: FnOnce(&I) -> R,
   |                            - help: consider further restricting the associated type: `, <I as intl_memoizer::Memoizable>::Args: std::marker::Send`
...
29 |             Ok(mut memoizer) => match memoizer.try_get(args) {
   |                                                ^^^^^^^ `<I as intl_memoizer::Memoizable>::Args` cannot be sent between threads safely
   |
   = help: the trait `std::marker::Send` is not implemented for `<I as intl_memoizer::Memoizable>::Args`

error: aborting due to 4 previous errors

which makes sense - our trait defines I: Memoizable + 'static. But I can't specialize it for Send+Sync in the concurrent impl because if I do, I get impl has extra requirementI: std::marker::Send``.

@Manishearth How can I specialize I separately for concurrent and non-concurrent variants?

[Manishearth]

We just need to move with_try_get to the memoizer impl itself

[Manishearth]

Figured it out

[Manishearth]

How does https://github.com/zbraniecki/fluent-rs/pull/2 look?

[zbraniecki]

Thank you! Re-requesting review with your patch applied!