-
Notifications
You must be signed in to change notification settings - Fork 380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ base ] Add non-blocking and timeout variants for channelGet #3435
Open
Matthew-Mosior
wants to merge
55
commits into
idris-lang:main
Choose a base branch
from
Matthew-Mosior:Issue-3424
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+136
−6
Open
Changes from 54 commits
Commits
Show all changes
55 commits
Select commit
Hold shift + click to select a range
a4aae0a
Initial implementation of non-blocking version of blodwen-channel-get.
Matthew-Mosior f261821
Adding channelGetNonBlocking function to System.Concurrency.idr (non-…
Matthew-Mosior 68ecf8b
Adding matching mutex-release call to initial mutex-acquire call.
Matthew-Mosior 89f7991
Fixing indentation in blodwen-channel-get-non-blocking.
Matthew-Mosior 9381947
Adding matching mutex-release call to initial mutex-acquire call (aga…
Matthew-Mosior a5610fa
Fixing return values for blodwen-channel-get-non-blocking.
Matthew-Mosior fb04471
Fixing extern type signature and channelGetNonBlocking function, alon…
Matthew-Mosior 9a53f9a
Removing blocking aspect from blodwen-channel-get-non-blocking.
Matthew-Mosior 818c5ba
Fixing styling of blodwen-channel-get-non-blocking.
Matthew-Mosior fe1dd2d
Removing extra mutex-release call.
Matthew-Mosior 8754137
Removing duplicate val calls.
Matthew-Mosior 7936ae1
Fixing return type.
Matthew-Mosior 45dfec5
Fixing return type.
Matthew-Mosior 21e877a
Fixing channelGetNonBlocking.
Matthew-Mosior 82defee
Reverting return type fix.
Matthew-Mosior af8f499
Removing unnecessary comments.
Matthew-Mosior dbf8e83
Remove incorrect documentation.
Matthew-Mosior fadb1e9
Altering blodwen-channel-get-non-blocking to error when mutex cant be…
Matthew-Mosior fb8fb26
Cleaning up commented out code.
Matthew-Mosior 2b6077b
Adding blodwen-channel-check scheme function to enable correct handli…
Matthew-Mosior f8c24af
Optimizing blodwen-channel-get-non-blocking.
Matthew-Mosior 9e7ba48
Changing Boolean comparision to Int (compatible with expected Prim ty…
Matthew-Mosior 0684a10
Adding decoding support for channelGetNonBlocking.
Matthew-Mosior 56d42bd
Removing extra %foreign pragmas.
Matthew-Mosior e8a39f3
Removing extra %foreign.
Matthew-Mosior 5a18386
Fixing blodwen-channel-get-non-blocking by removing extra set of pare…
Matthew-Mosior 6e8c4a5
Adding channelGetNonBlocking test.
Matthew-Mosior 0c4b2dd
Changing 0 to () for the return on failed mutex acquisition or an emp…
Matthew-Mosior 042799d
Removing extraneous whitespace.
Matthew-Mosior 636ca50
Removing unnecessary scheme decoding.
Matthew-Mosior 2d1b96c
Fixing initial channelGetNonBlocking test.
Matthew-Mosior 4a4699f
Fixing indentation for blodwen-channel-get-non-blocking.
Matthew-Mosior 5db18d8
Adding back decoding support, fixing blodwen-channel-get-non-blocking…
87265de
Adding another test for channelGetNonBlocking.
714e5b1
Removing tabs from blodwen-channel-get-non-blocking.
126a52a
Adding initial skeleton for blodwen-channel-get-with-timeout.
8d78679
Adding initial blodwen-channel-get-with-timeout function and associat…
Matthew-Mosior b3fa699
Fixing small issue with blodwen-channel-get-with-timeout.
8ee6e4b
Adding corrected blodwen-channel-get-with-timeout implementation and …
fdc11b6
Updating documentation for channelGetWithTimeout function.
98b485a
Adding Scheme Nat implementation and fixing test.
8eb7457
Re-ordering imports for channel007 test.
4f85729
Adding changes of this PR to CHANGELOG_NEXT.md.
80fafa9
Merge branch 'main' into Issue-3424
Matthew-Mosior 474bd6b
Fixing linting issues.
cd24d35
Adding additional sleep in channel007 test.
970ace4
Addressing comments.
0a79b46
Addressing comments (fixed).
6a5129f
Addressing comments (tests).
917b240
Fixing linting.
2a70bad
Addressing comments.
Matthew-Mosior 6b13dc8
Addressing @cypheon's comments.
Matthew-Mosior fa50df7
Removing duplicate tests from allscheme.
Matthew-Mosior ef41d27
Merge branch 'main' into Issue-3424
Matthew-Mosior eddaf30
Changing Nat to Int for FFI type on prim__channelGetWithTimeout.
Matthew-Mosior File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import System | ||
import System.Concurrency | ||
|
||
-- Test that using channelGetNonBlocking works as expected. | ||
main : IO () | ||
main = do | ||
chan <- makeChannel | ||
threadID <- fork $ do | ||
channelPut chan "Hello" | ||
channelPut chan "Goodbye" | ||
sleep 1 | ||
case !(channelGetNonBlocking chan) of | ||
Nothing => putStrLn "Nothing" | ||
Just val' => putStrLn val' | ||
sleep 1 | ||
case !(channelGetNonBlocking chan) of | ||
Nothing => putStrLn "Nothing" | ||
Just val' => putStrLn val' | ||
sleep 1 | ||
case !(channelGetNonBlocking chan) of | ||
Nothing => putStrLn "Nothing" | ||
Just val' => putStrLn val' | ||
sleep 1 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
Hello | ||
Goodbye | ||
Nothing |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
. ../../testutils.sh | ||
|
||
run Main.idr |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import Data.Maybe | ||
import System | ||
import System.Concurrency | ||
|
||
-- Simple producing thread. | ||
producer : Channel Nat -> Nat -> IO () | ||
producer c n = ignore $ producer' n | ||
where | ||
producer' : Nat -> IO () | ||
producer' Z = pure () | ||
producer' (S n) = do | ||
channelPut c n | ||
sleep 1 | ||
|
||
-- Test that channelGetWithTimeout works as expected. | ||
main : IO () | ||
main = | ||
do c <- makeChannel | ||
tids <- for [0..11] $ \n => fork $ producer c n | ||
vals <- for [0..11] $ \_ => channelGetWithTimeout c 5000 | ||
ignore $ traverse (\t => threadWait t) tids | ||
putStrLn $ show $ sum $ fromMaybe 0 <$> vals | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
55 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
. ../../testutils.sh | ||
|
||
run Main.idr |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last I checked
Nat
was not a blessed primitive type for passing via the FFI (https://idris2.readthedocs.io/en/latest/ffi/ffi.html#primitive-ffi-types). What that means in practice is that if it works then it "just happens to work" even thoughNat
may in practice be implemented in a way that is compatible withInt
for any given backend.Someone could correct me here, but best practice would be to use
Nat
in your exported code but cast toInt
for your FFI call.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattpolzin
Awesome catch here, I've addressed this via eddaf30
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattpolzin
It looks like all tests passed but one:
https://github.com/idris-lang/Idris2/actions/runs/12659854176/job/35280037440?pr=3435
It looks like it timed out, but not sure if that is supposed to happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't time out but that's not unheard of so I've just kicked tests off again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for kicking off the tests again, looks like it all passed this time.