-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add function to re initialize foreign type #47407
Conversation
@fingolfin Does this seem like a reasonable solution? |
But how would I declare |
Hm, probably |
If we can That said, I am still hoping to eliminate our use of foreign pointers on the top level... but that may or may not pan out, so I am happy if a second solution gets implemented |
So on the top level it'd be something like this: const GapObj = # <- is this even needed? or does `jl_new_datatype` take care of it?
ccall(:jl_new_foreign_type, Any,
(Any, Any, Any, Ptr, Ptr, Cint, Cint),
(:GapObj, @__MODULE__, Any, C_NULL, C_NULL, 1, 0))` And then in |
Would this solve #36770, or it's only part of it? |
Only part of it. Still no way to serialize instances |
I added the beginnings of a test that also illustrate how to use this. Not quite sure why |
For (de)serializing instances, what do you need @fingolfin? Can one just write the binary blob? (It is effectively |
I discussed this a bit at #46214. Short answer: no, it isn't a In fact, we currently use three foreign types:
|
Thank you, that's very helpful on multiple levels!!! |
[Not sure I understood "bother scanning" (typo for pointer scanning"?)] It's interesting to know such "tagged pointers" actually work in Julia (or well, until they didn't, with recent PR). I've been thinking of using such a trick (for a new string type, to store a prefix in the "pointer to String"). I know this would work in e.g. C, but hadn't thought through the issue with Julia or other GC languages (know tagged done in Lisp). It would be good to have documented which bits of a pointer you can or can't alter without screwing up GC, guaranteed for the future.
Is the tagged pointer this "foreign-datatype" (or at least what it points to)? It would be great if you can implement these (full) callbacks, if not already done (do I understand correctly that this PR, that I do not understand nor read carefully, does it?), and/or document somewhere what is already done or needs to be done. https://github.com/JuliaLang/Juleps/blob/master/GcExtensions.md
FYI: I do see: https://github.com/vchuravy/ForeignCallbacks.jl In my case my datatype wouldn't be foreign, in the sense of implemented in another language (I at least I hope it will be doable in Julia-only code), though I guess it could be done in C code, and it would be "foreign" in the sense of low-level C-like (or could as well be) Julia code. Maybe reconsider terminology to "tagged pointer type" from "foreign type"? |
Oops, the "bother" just can be removed, not sure what happened there (I probably edited back and forth and it's left over from some previous text). I've taken the liberty of editing a bit more afterwards to hopefully improve clarity.
I don't think they work! That's one of the reasons why we need to scan our data ourselves, instead of just specifying a "data layout": even if we know which bits are pointers and which are not, then the pointers still can be tagged... OFF-TOPIC: So the "tagged pointers" are a feature of GAP (and FLINT, and other systems), and they are really important for us in computational algebra, because we need arbitrary precision integers all the time, and tagged pointer integers compensate for the deficiencies of larger integer arithmetic as implemented by GMP -- i.e., while your integers are small, you don't need allocation and get performance close to "native"; and only when they grow you switch to a proper GMP integer (or similar). Indeed, for us, maybe the primary grievance about Julia is that it defaults to machine integers (which to us is "performance over correctness", and so a boo-boo). Anyway, I'd love if some limited form of tagged pointers like this was possible in Julia... i.e. something which is either, say, 60 bits of payload plus a few tag/guard bits; or else an Oh, and many GC languages have such tagged pointers, in various variations. In GAP, two LSB bits are used to mark non-pointers (this conveniently uses that our pointers are always multiplies of sizeof(void *), so on 32bit archs the lower two bits are always 0 for pointers; the three non-zero values are used to differentiate what kind of "immediate" value is stored in the remaining bits; of course these could then be handled differently depending on context; but in GAP, one of the patterns is dedicated for what we call "immediate integers": specificall bit 0 is set to 1; and bit 1 is set to 0; then 61 of the remaining 62 bits store a value; and finally the MSB, bit 63, is used as a "guard bit", i.e. bit 63 and bit 62 are always equal; this makes it cheap to implement efficient addition with overflow handling: just add the two immediate values unsigned; perform some bit manipulations; if an overflow occurred, discard and restart with a big int. For details see https://github.com/gap-system/gap/blob/master/src/intobj.h. Note that other implementations instead use top bits, esp. those which are 64bit only; they use that on many architectures, only 48 bits are really used for addresses, so you can store a ton of tags in the top bits. Thinking about Julia, one could have a The main benefit of that type would be that the Julia GC would know about it and treat it like END OFF TOPIC |
No, that would be wrong. The "foreign type" we use are not the same as our "tagged pointers" which we take great pains to hide from Julia. There is no "tagged pointer type" either. |
So, what needs to be done to move this from draft to get it merged? It sounds as if #44527 may be merged soon, at which point GAP (and hence OSCAR) would be broken in Julia nightly builds. From my perspective, the ideal situation would be if this PR was merged first, then I could immediately adapt to it and hopefully not even have any outage. One obvious thing that comes to mind is that I could try to adapt GAP.jl to the changes in this PR now, in an experimental branch of it, to validate the principle. Would that help? Is there anything else that ought to be done? |
If you can confirm that this suffices and work for you that would be ideal. @timholy and I have been a bit spread thin so I would be more than happy to hand this over to you for now :) |
7967b0f
to
5eb6195
Compare
@fingolfin I am not sure I understand why the test here doesn't work (nmark and nsweep) not being incremented.
Hm do you have an example I can test? They should be preserved... Make sure you run with |
No idea, I can try to look at that test, but right now I need to prepare for a lecture :-)
My testing set up is rather non-trivial, I afraid... But in a nutshell, I inserted printf statements to show the values of I am testing with a patched version of GAP plus GAP.jl, using the code in these PRs:
UPDATE: here are the new simplified instructions:
|
Just for completeness, after all that setup, I then see this (note the debug output):
This is with the stat of this PR as of now, i.e., commit 2ff47e6 |
@fingolfin figured out why the test was borked. I also now test that |
@vchuravy yet in my "real world" code, I've also updated to 4d1fd2c but no change. |
It's not during the restoration... It's seemingly during the serialization. @fingolfin seems like the problem is sitting in front of your monitor ;) Should probably have been: const GapObj = ccall(:jl_new_foreign_type, Any, (Symbol, Module, Any, Any, Any, Cint, Cint),
:GapObj, GAP, Any, C_NULL, C_NULL, 1, 0)
# TODO: can we "hide" the following two types? no Julia code should ever "touch" them,
# let alone see instances of them
const Bag = ccall(:jl_new_foreign_type, Any, (Symbol, Module, Any, Any, Any, Cint, Cint),
:Bag, GAP, Any, C_NULL, C_NULL, 1, 0)
const LargeBag = ccall(:jl_new_foreign_type, Any, (Symbol, Module, Any, Any, Any, Cint, Cint),
:LargeBag, GAP, Any, C_NULL, C_NULL, 1, 1) |
Also note that I moved the call to |
Aaargh, indeed (sadly not the first time, either sigh). Great then all seems to work. Thank you again for working on this and your patience. I'll clean up my various PRs now. Yeah, I only put the The most annoying part now will be to structure the code so that it'll keep working as before on "older" Julia versions but use the new conventions otherwise. Hurm...
|
Co-authored-by: Tim Holy <tim.holy@gmail.com> Co-authored-by: Max Horn <max@quendi.de>
96f6008
to
0cc2ce9
Compare
I split out the get the tests working commits into #47699 From my perspective this is good to go. |
Co-authored-by: Tim Holy <tim.holy@gmail.com> Co-authored-by: Max Horn <max@quendi.de>
Co-authored-by: Tim Holy <tim.holy@gmail.com> Co-authored-by: Max Horn <max@quendi.de>
Co-authored-by: Tim Holy <tim.holy@gmail.com> Co-authored-by: Max Horn <max@quendi.de>
Co-authored-by: Tim Holy <tim.holy@gmail.com> Co-authored-by: Max Horn <max@quendi.de> (cherry picked from commit e06a591)
For foreign types support in GC to work in PackageCompiler and the
.ji
format rewrite,we need to declare the types at the module top-level, but after module loading we will
need to re-initialize the hidden
desc
field with the runtime pointers of the markand sweep function.