%->% signal forwarder #23
Conversation
|
@wlandau I'll wait for you to do a bit of testing on this, see if this is up to the job. |
|
Thank you so much, @shikokuchuo! Works great! > library(nanonext)
> cv1 <- cv()
> cv2 <- cv()
>
> cv_value(cv1)
[1] 0
> cv_value(cv2)
[1] 0
>
> cv1 %->% cv2
> cv_signal(cv1)
>
> cv_value(cv1)
[1] 1
> cv_value(cv2)
[1] 1 |
|
Hmm... odd things are happening with library(nanonext)
cv1 <- cv()
cv2 <- cv()
cv1 %->% cv2
cv_signal(cv1)
cv_signal(cv2)
wait(cv1)So far so good: > cv_value(cv1)
[1] 0
> cv_value(cv2)
[1] 2Then later I signal the first one: cv_signal(cv1)Should the value of > cv_value(cv1)
[1] 1
> cv_value(cv2)
[1] 2 |
|
I guess this won't affect my case because I will be waiting on CV2, not CV1. This works as expected: > library(nanonext)
> cv1 <- cv()
> cv2 <- cv()
> cv_signal(cv1)
> cv1 %->% cv2
> cv_signal(cv1)
> cv_signal(cv2)
> wait(cv2)
> cv_value(cv1)
[1] 2
> cv_value(cv2)
[1] 1
> cv_signal(cv1)
> cv_value(cv1)
[1] 3
> cv_value(cv2)
[1] 2 |
|
I think this is precisely the kind of thing I was referring to in #20. Condition variables are essentially control flow constructs, and it just so happens that they can also be used as fancy counters. But I don't think you can overcome the race conditions to use them as both. |
|
I still think this PR is fantastic. Because of #23 (comment), I think I can use an upstream CV as a fancy counter, a downstream CV as a control flow construct, and forward the upstream CV to the downstream CV using |
|
By the way, this PR works perfectly as a solution to #21 for library(nanonext)
downstream <- cv()
upstream <- replicate(3L, cv())
purrr::walk(upstream, ~.x %->% downstream)
cv_value(downstream)
#> [1] 0
purrr::walk(upstream, ~cv_signal(.x))
cv_value(downstream)
#> [1] 3 |
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #23 +/- ##
==========================================
+ Coverage 87.19% 87.24% +0.05%
==========================================
Files 22 22
Lines 3388 3442 +54
==========================================
+ Hits 2954 3003 +49
- Misses 434 439 +5 ☔ View full report in Codecov by Sentry. |
|
Needs to be renamed, clashes with the destrucuturing assignment operator in |
|
I would be okay if this were only a basic function instead of an operator. I think the case for an operator might be if you have long chains of forwarded CVs, which seems like a rare use case. |
|
Thanks for the suggestion, I have changed I need a bit more time to convince myself that it's stable enough to ship - I'm having trouble getting deterministic test results given the async nature of the forwarding. |
|
Think I've solved this (checks pass). For safety, 'cv' is now reset to zero by the operator, but I am guessing shouldn't be a problem for the intended use cases. Also as documented, 'cv' only to be used as a counter with all waits on 'cv2'. @wlandau let me know if this looks good on your side. |
|
My tests from previous comments still work as expected, but I am having trouble under load on Mac OS. It seems not every signal gets forwarded: library(nanonext)
cv1 <- cv()
cv2 <- cv()
cv1 %~>% cv2
tmp <- replicate(1e3, cv_signal(cv1))
cv_value(cv1)
#> [1] 1000
cv_value(cv2)
#> [1] 335Created on 2023-11-15 with reprex v2.0.2 Session infosessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#> setting value
#> version R version 4.3.2 (2023-10-31)
#> os macOS Sonoma 14.1
#> system aarch64, darwin20
#> ui X11
#> language (EN)
#> collate en_US.UTF-8
#> ctype en_US.UTF-8
#> tz America/Indiana/Indianapolis
#> date 2023-11-15
#> pandoc 3.1.2 @ /usr/local/bin/ (via rmarkdown)
#>
#> ─ Packages ───────────────────────────────────────────────────────────────────
#> package * version date (UTC) lib source
#> cli 3.6.1 2023-03-23 [1] CRAN (R 4.3.0)
#> digest 0.6.33 2023-07-07 [1] CRAN (R 4.3.0)
#> evaluate 0.22 2023-09-29 [1] CRAN (R 4.3.1)
#> fastmap 1.1.1 2023-02-24 [1] CRAN (R 4.3.0)
#> fs 1.6.3 2023-07-20 [1] CRAN (R 4.3.0)
#> glue 1.6.2 2022-02-24 [1] CRAN (R 4.3.0)
#> htmltools 0.5.6.1 2023-10-06 [1] CRAN (R 4.3.1)
#> knitr 1.44 2023-09-11 [1] CRAN (R 4.3.0)
#> lifecycle 1.0.3 2022-10-07 [1] CRAN (R 4.3.0)
#> magrittr 2.0.3 2022-03-30 [1] CRAN (R 4.3.0)
#> nanonext * 0.10.4.9001 2023-11-15 [1] Github (shikokuchuo/nanonext@35569cb)
#> purrr 1.0.2 2023-08-10 [1] CRAN (R 4.3.0)
#> R.cache 0.16.0 2022-07-21 [1] CRAN (R 4.3.0)
#> R.methodsS3 1.8.2 2022-06-13 [1] CRAN (R 4.3.0)
#> R.oo 1.25.0 2022-06-12 [1] CRAN (R 4.3.0)
#> R.utils 2.12.2 2022-11-11 [1] CRAN (R 4.3.0)
#> reprex 2.0.2 2022-08-17 [1] CRAN (R 4.3.0)
#> rlang 1.1.1 2023-04-28 [1] CRAN (R 4.3.0)
#> rmarkdown 2.25 2023-09-18 [1] CRAN (R 4.3.1)
#> rstudioapi 0.15.0 2023-07-07 [1] CRAN (R 4.3.0)
#> sessioninfo 1.2.2 2021-12-06 [1] CRAN (R 4.3.0)
#> styler 1.10.2 2023-08-29 [1] CRAN (R 4.3.0)
#> vctrs 0.6.4 2023-10-12 [1] CRAN (R 4.3.1)
#> withr 2.5.1 2023-09-26 [1] CRAN (R 4.3.1)
#> xfun 0.40 2023-08-09 [1] CRAN (R 4.3.0)
#> yaml 2.3.7 2023-01-23 [1] CRAN (R 4.3.0)
#>
#> [1] /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/library
#>
#> ────────────────────────────────────────────────────────────────────────────── |
|
Getting similar results on a RHEL7 machine. |
|
The fix for the load tests changes the behaviour slightly - as newly documented: ?`%~>%`Hopefully that is clear. Should still serve the original purpose, but much more robust now. |
|
That test now passes for me too. From my perspective, everything looks good to merge. Here is a more complicated test which collects forwarded signals while those signals are being sent. It also passes: # test
library(nanonext)
cv1 <- cv()
cv2 <- cv()
cv1 %~>% cv2
n <- 1000
signalled <- rep(NA, n)
for (index in seq_along(signalled)) {
cv_signal(cv1)
signalled[index] <- until(cv2, 0)
}
signals <- print(sum(signalled))
#> [1] 989
# results
signals
#> [1] 989
cv_value(cv2)
#> [1] 11
signals + cv_value(cv2) == n
#> [1] TRUE
cv_value(cv1)
#> [1] 1000Created on 2023-11-16 with reprex v2.0.2 Session infosessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#> setting value
#> version R version 4.3.2 (2023-10-31)
#> os macOS Sonoma 14.1
#> system aarch64, darwin20
#> ui X11
#> language (EN)
#> collate en_US.UTF-8
#> ctype en_US.UTF-8
#> tz America/Indiana/Indianapolis
#> date 2023-11-16
#> pandoc 3.1.2 @ /usr/local/bin/ (via rmarkdown)
#>
#> ─ Packages ───────────────────────────────────────────────────────────────────
#> package * version date (UTC) lib source
#> cli 3.6.1 2023-03-23 [1] CRAN (R 4.3.0)
#> digest 0.6.33 2023-07-07 [1] CRAN (R 4.3.0)
#> evaluate 0.22 2023-09-29 [1] CRAN (R 4.3.1)
#> fastmap 1.1.1 2023-02-24 [1] CRAN (R 4.3.0)
#> fs 1.6.3 2023-07-20 [1] CRAN (R 4.3.0)
#> glue 1.6.2 2022-02-24 [1] CRAN (R 4.3.0)
#> htmltools 0.5.6.1 2023-10-06 [1] CRAN (R 4.3.1)
#> knitr 1.44 2023-09-11 [1] CRAN (R 4.3.0)
#> lifecycle 1.0.3 2022-10-07 [1] CRAN (R 4.3.0)
#> magrittr 2.0.3 2022-03-30 [1] CRAN (R 4.3.0)
#> nanonext * 0.10.4.9001 2023-11-16 [1] Github (shikokuchuo/nanonext@43c30be)
#> purrr 1.0.2 2023-08-10 [1] CRAN (R 4.3.0)
#> R.cache 0.16.0 2022-07-21 [1] CRAN (R 4.3.0)
#> R.methodsS3 1.8.2 2022-06-13 [1] CRAN (R 4.3.0)
#> R.oo 1.25.0 2022-06-12 [1] CRAN (R 4.3.0)
#> R.utils 2.12.2 2022-11-11 [1] CRAN (R 4.3.0)
#> reprex 2.0.2 2022-08-17 [1] CRAN (R 4.3.0)
#> rlang 1.1.1 2023-04-28 [1] CRAN (R 4.3.0)
#> rmarkdown 2.25 2023-09-18 [1] CRAN (R 4.3.1)
#> rstudioapi 0.15.0 2023-07-07 [1] CRAN (R 4.3.0)
#> sessioninfo 1.2.2 2021-12-06 [1] CRAN (R 4.3.0)
#> styler 1.10.2 2023-08-29 [1] CRAN (R 4.3.0)
#> vctrs 0.6.4 2023-10-12 [1] CRAN (R 4.3.1)
#> withr 2.5.1 2023-09-26 [1] CRAN (R 4.3.1)
#> xfun 0.40 2023-08-09 [1] CRAN (R 4.3.0)
#> yaml 2.3.7 2023-01-23 [1] CRAN (R 4.3.0)
#>
#> [1] /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/library
#>
#> ────────────────────────────────────────────────────────────────────────────── |
initial implementation