-
Notifications
You must be signed in to change notification settings - Fork 132
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
Make some enhancements to atomic integers #776
Draft
Kaiepi
wants to merge
24
commits into
Raku:main
Choose a base branch
from
Kaiepi:counter
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.
Draft
Conversation
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
Originally from the ill-fated semaphoverhaul branch, it would be nice if `!` worked on `uint`.
This involves a minimal port of Rakudo's `Perl6::Metamodel::NativeRefHOW`. Because the `Ref` types from NQP should not appear in user code, treat it a little more like a knowhow than a native supporting inheritance. Past that point, it's more or less a copy paste job, just we need our SC write barriers when setting their container specs up.
The metamodel needs a reasonably fast thread-safe counter. Atomic ops are also easy to screw up, on the other hand. This adds a `Counter` class to `NQPCORE.setting` to take care of this for ourselves, which performs comparably to the semaphore that it was originally intended to be. This probably doesn't work on backends lacking native atomic integer support. *Surely* we have a version of the JVM and node.js that makes this possible by now?? Right???
Because there's a change of state necessary when the predecessor is 0.
Counter could use this in order to coerce from an IntStr, for instance.
It can be `Cool` instead of `Any` in Raku land, but it needs a bit of boilerplate here.
The internal atomics need to be at their core and can remain an `int` up until the point where they are exposed to the user.
- Former CALL-ME is now COERCE - CALL-ME dispatches to the quasi-tree that exists in its method table for getters, since users can and will try to exploit patterns in their method names when `_` is apt to become `-` some day. There is a performance tradeoff here; don't depend on this if you know the full method name.
Apparently, on POSIX platforms, `int` and `atomicint` are equivalent in the VM's eyes, but not on Windows. We need a native integer with the correct size for atomicops for `Counter`.
Though we don't depend on this capability ourselves, a user may be interested in subtyping `Counter` to introduce a getter with arguments in some form.
This used to be depended on in a few places while I was trying to see if `Positional` or `Associative` would work (`CALL-ME` has aspects of `AT-POS`/`AT-KEY`, but fulfill neither), but now only one candidate is concerned about summoning a method name from a list of operations.
The value of the counter can be *above* `-1` here, dummy. Do a numeric equality check on the upper bound like everywhere else.
- Delete unused register - Remove erroneous change to an unnecessary method call in int64's handling.
i.e. the `$!blocker` flag *always* needs to be set to reflect changes to the `$!counter`.
Because `drop_succ` doesn't need it either.
The JVM and its new atomicint ops can't build if anything depends on them here before rebootstrapping.
- Third argument must be `nqp::null_s` here. - Missing semicolon breaks builds altogether.
Unfortunately, we will wind up with more of these, since we have them to begin with because of atomic ops. We've gotten some release version bumps since writing, so there's a way to write such an interface (`org.raku.nqp.sixmodel.runtime.Intrinsics` and its `butUnsafe`/`itsUnsafe` static methods) now (with lambdas).
For some godforsaken reason, Java has atomic integers without a standard means of fencing. Luckily, an internal means of doing so is exposed in the `Unsafe` singleton.
We use Java 9 now, so we can get rid of some of the locking involved here.
As commented, VarHandle can handle the atomic ops that we would've been forced to depend on Unsafe to accomplish (racily!) prior to Java 9.
We should probably use `-Xlint:all` some day, but: 1. There are a lot more warnings to go. 2. This is supposed to be part of an atomics PR. So just focus on the warnings that are visible by default for now. Lint for these to prevent them from popping up again.
- The name of a class probably winds up with a similar lifetime to the class itself. Make the key references weak too with `WeakReference` (which is weaker than the prior `SoftReference`, which errs to keeping references alive). - Synchronize the entire chain of hash operations involved in retrieving a cached class, rather than the operations individually.
It would be really nice if someone could continue the work of Kaiepi |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Counter
that performs comparably.Counter
needs sanity tests in its own right. Swapping outSemaphore
and itsacquire
/release
for aCounter
and itswant_pred
/need_succ
int/spec/S17-lowlevel/semaphore.t
passes OK.