Strategy does not implement Sync

[vitiral]

I'm not sure if there is a technical reason this is not possible, but I thought I would check.

I would like to create the regex strategy using lazy_static! but Strategies don't implement Sync.

Thanks!

error[E0277]: the trait bound `proptest::strategy::Strategy<Value=std::boxed::Box<proptest::strategy::ValueTree<Value=std::string::String> + 'static>> + 'static: std::marker::Sync` is not satisfied
  --> src/test_name.rs:38:1
   |
38 | / lazy_static!{
39 | |     static ref GEN_NAME_PROP: prop::string::RegexGeneratorStrategy<String> =
40 | |         prop::string::string_regex(GEN_NAME_RE).unwrap();
41 | | }
   | |_^ `proptest::strategy::Strategy<Value=std::boxed::Box<proptest::strategy::ValueTree<Value=std::string::String> + 'static>> + 'static` cannot be shared between threads safely
   |
   = help: the trait `std::marker::Sync` is not implemented for `proptest::strategy::Strategy<Value=std::boxed::Box<proptest::strategy::ValueTree<Value=std::string::String> + 'static>> + 'static`
   = note: required because of the requirements on the impl of `std::marker::Sync` for `std::ptr::Unique<proptest::strategy::Strategy<Value=std::boxed::Box<proptest::strategy::ValueTree<Value=std::string::String
> + 'static>> + 'static>`
   = note: required because it appears within the type `std::boxed::Box<proptest::strategy::Strategy<Value=std::boxed::Box<proptest::strategy::ValueTree<Value=std::string::String> + 'static>> + 'static>`
   = note: required because it appears within the type `proptest::string::RegexGeneratorStrategy<std::string::String>`
   = note: required by `lazy_static::lazy::Lazy`
   = note: this error originates in a macro outside of the current crate

error: aborting due to previous error

error: Could not compile `artifact-data`.

[vitiral]

also, any advice on speeding up generation of random strings? I'm seeing significant (i.e. 10x) slowdowns compared to quickcheck + regex_generate.

[vitiral]

one thing I did: reduce the number generated for tests that don't need a large number. Very hard to do with quickcheck!

[AltSysrq]

Strategies don't implement Sync

I originally wanted to leave the possibility of using interior mutability within a strategy. But as I found out trying to implement prop_recursive with it, you can't usefull track state in a strategy anyway, so I don't think there's any reason not to add Sync at this point.

seeing significant (i.e. 10x) slowdowns compared to quickcheck + regex_generate

That's somewhat expected. Proptest needs to build and retain a more complicated structure for shrinking to work correctly. The generation could also benefit a lot from small string optimisation; currently, each individual element of the regex becomes its own Vec<u8>, so you may be paying for an enormous amount of allocations depending on the regex. I'll look into improving this soon.

[vitiral]

Thanks for the replies. It would be nice to add Sync.

As for the slowdown, I think it's mostly fine -- obviously faster is always better :)

It would probably be worth documenting eventually though, especially if a solution cannot be found.

[AltSysrq]

It turns out that adding Strategy: Send + Sync would actually be pretty painful as combinators like prop_map would need to require the closures to be Send + Sync. Instead, I'm just fixing the immediate problem which was specific to the regex strategy, which used Box<Strategy> under the hood and so erased the OIBITS. I'm also adding a conventional way to make a Box<Strategy + Send + Sync> so external users can get the same benefit.

The only other remaining internal use of BoxedStrategy is in prop_recursive(), but I'm not sure what could be done there since switching to the Send + Sync version would restrict how people could use that combinator.

[AltSysrq]

@vitiral Would you be able to provide the regex in question? I started looking into optimising the string generation, but the regex I tried ([0-9]{4}-[0-9]{2}-[0-9]{2} .*) ended up being faster than I anticipated (~9μs per iteration), so it would help to have a slow-but-realistic test case.

[vitiral]

Here is the regex I am using:

const GEN_NAME_RE: &str = r#"(?ix)
(REQ|SPC|TST)-              # the type followed by `-`
([a-z0-9_]{1,20}-){0,8}     # an optional number of `elem-` elements
[a-z0-9_]{1,20}             # required final element
"#;

[vitiral]

originally I was using * instead of the ranges {1,20} or {0,8}. That was slow, so I adjusted the ranges to (I think) up to 50 and then down to 20/8.

Hope that helps!

[AltSysrq]

The various string-generation strategies are now Send + Sync as of 0.3.2.

I also did look into the performance of string generation, and I believe the main bottleneck is actually character generation due to the inefficient way it accomplishes biasing towards particular characters. Changing that will likely require breaking backwards compatibility, so I'll make another pass at that at a later time.

[AltSysrq]

I've moved the regex performance concerns to a separate issue (#16) since the problem referenced by the title of this one has been fixed.

[vitiral]

Thanks! Wow, you are really responsive on these issues. Hope they were helpful for you as a lib designer!