Skip to content
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

introduce lock, unlock, islocked, isready, get! and deprecate get, request, release #93

Merged
merged 13 commits into from
Aug 2, 2023

Conversation

Krastanov
Copy link
Member

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2023

Codecov Report

Merging #93 (718f1db) into master (91a9809) will increase coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #93      +/-   ##
==========================================
+ Coverage   98.06%   98.17%   +0.11%     
==========================================
  Files          10       10              
  Lines         258      274      +16     
==========================================
+ Hits          253      269      +16     
  Misses          5        5              
Files Changed Coverage Δ
src/ConcurrentSim.jl 100.00% <ø> (ø)
src/resources/containers.jl 100.00% <100.00%> (ø)
src/resources/stores.jl 95.23% <100.00%> (+1.12%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Krastanov
Copy link
Member Author

This takes care of some of the points discussed in #91.

In particular, it does NOT implement trylock, nor does it deprecate get in favor of pop!.

No functionality is removed, but request, release, put will now show warnings (only in tests, not in usual use). I do not expect these deprecated functions to actually be removed (basically ever).

The failure of the tests on nightly does not seem related to this PR.

I will plan to merge in a few days unless someone requests more time for review.

@Krastanov
Copy link
Member Author

the nightly failures are due to JuliaLang/julia#50710

@Krastanov
Copy link
Member Author

now take! and trylock are also implemented, finishing the rest of #91

#92 and #86 are also now fixed

@Krastanov Krastanov requested a review from hdavid16 July 29, 2023 04:57
Copy link
Member

@hdavid16 hdavid16 left a comment

Choose a reason for hiding this comment

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

Looks good. So essentially, you are treating resources as if they were Julia channels and extending the base functions for these. Are there any benefits besides renaming the functions? How does this help integrating with other packages if resources are not Channels?

@Krastanov
Copy link
Member Author

Resources/Containers are treated as locks and Stores are treaded as channels. The value is that now I can have a downstream library that can switch between ConcurrentSim and controlling real devices / networks, with almost the same API (the real device / network controller also uses locks and channels). Not everything translates well yet, e.g. I still need to do a lot of testing to ensure multithreading works as expected, etc. but that is a convenient first step.

@Krastanov
Copy link
Member Author

Although, I am not 100% sure on this yet, so I will not be merging it until I have the downstream library actually do all the stuff I described in the previous comment.

@Krastanov Krastanov mentioned this pull request Jul 29, 2023
@hdavid16
Copy link
Member

Sounds like a good use case for this change.

@Krastanov
Copy link
Member Author

@hdavid16 , I undid some of the deprecations. All the new features are still in, but lock/trylock do not exist anymore, rather it is back to request (and a new tryrequest). The reason for undoing this is that lock is supposed to block, unlike request which is to be yielded upon. In the future we might be able to take advantage of the promises/futures objects to simplify this further.

@Krastanov Krastanov merged commit c62a6e5 into master Aug 2, 2023
7 of 8 checks passed
@Krastanov
Copy link
Member Author

I merged without squashing because each commit was a relatively clearly separate steps. In case we need to undo / cherrypick a single change, this should make it a bit easier.

@Krastanov Krastanov deleted the convenienceinterfaces branch June 19, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants