Skip to content

Correctly set and restore the TLV and remove the Cargo feature for it#2

Closed
Zoxc wants to merge 9 commits intorust-lang:rustcfrom
Zoxc:fix-tlv
Closed

Correctly set and restore the TLV and remove the Cargo feature for it#2
Zoxc wants to merge 9 commits intorust-lang:rustcfrom
Zoxc:fix-tlv

Conversation

@Zoxc
Copy link
Copy Markdown

@Zoxc Zoxc commented Apr 22, 2019

For some reason I didn't think to restore the TLV after running other Rayon jobs.

r? @cuviper / @nikomatsakis

Comment thread rayon-core/Cargo.toml Outdated

[features]
default = ["tlv"]
tlv = []
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Isn't removing a feature backwards-incompatible?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Probably, rustc-rayon doesn't provide any guarantees though.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Still, might as well bump to "0.2.0" I would think?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(At least that's what I usually do)

@nikomatsakis
Copy link
Copy Markdown

I'm 👍 to merge but would prefer to set version to 0.2.0 first

@Zoxc Zoxc force-pushed the rustc branch 2 times, most recently from da4fbb8 to 4b574ba Compare April 25, 2019 20:46
@Zoxc
Copy link
Copy Markdown
Author

Zoxc commented Apr 25, 2019

Merged and published 0.2.0.

@Zoxc Zoxc closed this Apr 25, 2019
nikomatsakis pushed a commit that referenced this pull request Oct 7, 2019
These adaptors consume may many elements before deferring to a base
folder's fullness checks, and so they need to be performed
manually. For the `filter`s, there's no way to do it manually (rayon-rs#632),
so the specialisations just have to be removed. For `fold` and
`find_any` this can be done with a `take_while`.

This extends the octillion tests to confirm this behaviour.

This makes a program like the following slightly slower compared to
the direct `consume_iter` without a check, but it's still faster than
the non-specialized form.

```
extern crate test;
extern crate rayon;
use rayon::prelude::*;

fn main() {
    let count = (0..std::u32::MAX)
        .into_par_iter()
        .map(test::black_box)
        .find_any(|_| test::black_box(false));
    println!("{:?}", count);
}
```

```
$ hyperfine ./find-original ./find-no-check ./find-check
Benchmark #1: ./find-original
  Time (mean ± σ):     627.6 ms ±  25.7 ms    [User: 7.130 s, System: 0.014 s]
  Range (min … max):   588.4 ms … 656.4 ms    10 runs

Benchmark #2: ./find-no-check
  Time (mean ± σ):     481.5 ms ±  10.8 ms    [User: 5.415 s, System: 0.013 s]
  Range (min … max):   468.9 ms … 498.2 ms    10 runs

Benchmark #3: ./find-check
  Time (mean ± σ):     562.3 ms ±  11.8 ms    [User: 6.363 s, System: 0.013 s]
  Range (min … max):   542.5 ms … 578.2 ms    10 runs
```

(find-original = without specialization, find-no-check = custom
`consume_iter` without `take_while`, find-check = this commit)
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.

3 participants