search

smol-rs / async-broadcast

Async broadcast channels
Apache License 2.0
151 stars 26 forks source link

Use parking_lot RwLock instead of std::sync::Mutex #27

Closed Antiarchitect closed 2 years ago

Antiarchitect commented 2 years ago

Hi, benches show that it slightly reduces the performance in 1 -> 1 case, but in 1 -> n cases performance has increased by about 10-20%. The purpose of the broadcast channel is to handle 1 -> n cases well.

master results:

1 -> 1                  time:   [57.567 ns 57.669 ns 57.781 ns]
Found 10 outliers among 100 measurements (10.00%)
  6 (6.00%) high mild
  4 (4.00%) high severe

1 -> 2                  time:   [109.41 ns 109.95 ns 110.76 ns]
Found 9 outliers among 100 measurements (9.00%)
  4 (4.00%) high mild
  5 (5.00%) high severe

1 -> 4                  time:   [162.51 ns 162.92 ns 163.40 ns]

1 -> 8                  time:   [263.26 ns 263.90 ns 264.67 ns]
Found 15 outliers among 100 measurements (15.00%)
  7 (7.00%) high mild
  8 (8.00%) high severe

parking_lot RwLock result:

1 -> 1                  time:   [68.974 ns 69.136 ns 69.326 ns]
                        change: [+18.780% +19.893% +21.127%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 10 outliers among 100 measurements (10.00%)
  4 (4.00%) high mild
  6 (6.00%) high severe

1 -> 2                  time:   [95.413 ns 95.889 ns 96.401 ns]
                        change: [-13.716% -13.242% -12.811%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

1 -> 4                  time:   [136.00 ns 136.32 ns 136.67 ns]
                        change: [-17.531% -16.911% -16.222%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  7 (7.00%) high mild
  3 (3.00%) high severe

1 -> 8                  time:   [231.09 ns 233.31 ns 235.76 ns]
                        change: [-13.582% -12.924% -12.295%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
  7 (7.00%) high mild
  4 (4.00%) high severe
zeenix commented 2 years ago

It's always good to have rationale on each commit.

Apart that these nits, LGTM otherwise. @yoshuawuyts do you have any opinions on using parking_lot?

zeenix commented 2 years ago

It's always good to have rationale on each commit.

Still no rationale on the commit. :( Now that it's just one commit, you can just copy&paste the PR description on it.

Antiarchitect commented 2 years ago

@zeenix Sorry for the misunderstanding. Will do it right away.

zeenix commented 2 years ago

@zeenix Sorry for the misunderstanding.

No problem. :)

Will do it right away.

Many thanks.

zeenix commented 2 years ago

ughh.. not sure why GH doesn't indicate warnings on the main page. :(

lint missing_doc_code_examples has been renamed to rustdoc::missing_doc_code_examples

@Antiarchitect I don't know why you removed the good fix for this instead of splitting it into its own commit, as I suggested. :(

Antiarchitect commented 2 years ago

@zeenix Sorry, I'm tired today :). Will do new MR for this :)