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

Changes to support moving immix #27

Merged
merged 12 commits into from
Feb 12, 2024
Merged

Conversation

udesou
Copy link

@udesou udesou commented Aug 22, 2023

Opening this draft PR to keep track of the changes to support moving immix. So far they are:
(1) pinning objects that are hashed. Note that Julia hashes on the address of the object, which means that we might get errors if we move an object that has been hashed.
(2) pinning buffer objects: these can be shared between objects, and there are some invariants that should be enforced (if a->is_shared is set to true, I believe we cannot move the buffer), to be on the safe side, we're pinning these objects.
(3) pinning types and typenames: these are used when scanning objects, and not traced from the objects as they are stored in the objects' header.
For (1), until we implement address space hashing, we need to make sure these objects do not move. For (2) and (3), we should be simply allocate these objects in a non-moving space.
There are also small bug fixes. Eg, Julia does not put a reference on the shadow stack, because it comes from a global root. In that case, if that object moves, the pointer to it is not updated. Finally I've also added code to clear the stack of dead tasks, so I have removed the static attribute from free_stack and _jl_free_stack in order to reuse these functions from MMTk.

NB: merge after mmtk/mmtk-core#897 in order to support transitively pinned roots.

@udesou udesou requested a review from qinsoon August 23, 2023 06:20
qinsoon pushed a commit that referenced this pull request Sep 1, 2023
It's possible for PiNodes to effectively imply statically the condition
of a Core.ifelse. For example:
```julia
    23 ─ %60  = Core.ifelse(%47, false, true)::Bool
    │    %61  = Core.ifelse(%47, %58, false)::Union{Missing, Bool}
    25 ─        goto #27 if not %60
    26 ─ %65  = π (%61, Bool)
    └───        ...
```

In basic block #26, the PiNode gives us enough information to conclude
that `%47 === false` if control flow reaches that point. The previous
code incorrectly assumed that this kind of pruning would only be done
for PhiNodes.

Resolves JuliaLang#50276
@qinsoon qinsoon marked this pull request as ready for review September 13, 2023 05:56
Copy link
Member

@qinsoon qinsoon left a comment

Choose a reason for hiding this comment

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

LGTM

src/threading.c Outdated Show resolved Hide resolved
src/julia_internal.h Show resolved Hide resolved
src/builtins.c Outdated
@@ -392,6 +395,9 @@ static uintptr_t immut_id_(jl_datatype_t *dt, jl_value_t *v, uintptr_t h) JL_NOT
return ~h;
size_t f, nf = jl_datatype_nfields(dt);
if (nf == 0 || (!dt->layout->haspadding && dt->layout->npointers == 0)) {
if(mmtk_object_is_managed_by_mmtk(v)) {
mmtk_pin_object(v);
Copy link
Member

Choose a reason for hiding this comment

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

For mmtk/mmtk-julia#104, probably we will introduce jl_gc_pin or something like that. But it is fine for now.

@udesou udesou merged commit 861f151 into mmtk:master Feb 12, 2024
1 check passed
udesou added a commit to mmtk/mmtk-julia that referenced this pull request Feb 12, 2024
This PR adds support for (partially) moving objects in Julia and should
be merged with mmtk/julia#27 and
mmtk/mmtk-core#897.
- It adds support for pinning/unpinning objects and checking if an
object is pinned (the implementation uses a pin bit).
(`mmtk_pin_object`, `mmtk_unpin_object` and `mmtk_is_pinned`)
- It adds support for providing transitively pinned (`tpinned`) roots .
- It implements the `copy` function in `object_model.rs`. Note that
arrays with inlined data must be treated specially, as their `a->data`
pointer needs to be updated after copying.
- It uses Julia's GC bits to store forwarding information and the
object's header to store the forwarding pointer.
- Currently, all stack roots are transitively pinned. Note that we also
need to traverse the `tls->live_tasks` to make sure that any stack root
from these tasks are transitively pinned.
- `scan_julia_object` had to be adapted to cover a few corner cases:
- when an array object contains a pointer to the owner of the data,
`a->data` needs to be updated in case the owner moves.
- the `using` field inside a `jl_module_t` may also be inlined inside
the module, and if that's the case, we need to make sure that field is
updated if the module moves.
- when marking finalizers, traversing the list of malloced arrays, and
the list of live tasks at the end of GC, we need to updated these lists
with objects that have possibly been moved.
- Added a few debug assertions to capture scanning of misaligned objects
and roots.

NB: I've only tested moving immix; sticky immix is still non-moving.

---------

Co-authored-by: Luis Eduardo de Souza Amorim <eduardo@shrew.moma>
Co-authored-by: Luis Eduardo de Souza Amorim <eduardo@bear.moma>
Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
mergify bot pushed a commit to mmtk/mmtk-julia that referenced this pull request Feb 12, 2024
This PR adds support for (partially) moving objects in Julia and should
be merged with mmtk/julia#27 and
mmtk/mmtk-core#897.
- It adds support for pinning/unpinning objects and checking if an
object is pinned (the implementation uses a pin bit).
(`mmtk_pin_object`, `mmtk_unpin_object` and `mmtk_is_pinned`)
- It adds support for providing transitively pinned (`tpinned`) roots .
- It implements the `copy` function in `object_model.rs`. Note that
arrays with inlined data must be treated specially, as their `a->data`
pointer needs to be updated after copying.
- It uses Julia's GC bits to store forwarding information and the
object's header to store the forwarding pointer.
- Currently, all stack roots are transitively pinned. Note that we also
need to traverse the `tls->live_tasks` to make sure that any stack root
from these tasks are transitively pinned.
- `scan_julia_object` had to be adapted to cover a few corner cases:
- when an array object contains a pointer to the owner of the data,
`a->data` needs to be updated in case the owner moves.
- the `using` field inside a `jl_module_t` may also be inlined inside
the module, and if that's the case, we need to make sure that field is
updated if the module moves.
- when marking finalizers, traversing the list of malloced arrays, and
the list of live tasks at the end of GC, we need to updated these lists
with objects that have possibly been moved.
- Added a few debug assertions to capture scanning of misaligned objects
and roots.

NB: I've only tested moving immix; sticky immix is still non-moving.

---------

Co-authored-by: Luis Eduardo de Souza Amorim <eduardo@shrew.moma>
Co-authored-by: Luis Eduardo de Souza Amorim <eduardo@bear.moma>
Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
(cherry picked from commit 1622162)

# Conflicts:
#	julia/mmtk_julia.c
#	mmtk/Cargo.lock
#	mmtk/Cargo.toml
#	mmtk/api/mmtk.h
#	mmtk/src/julia_scanning.rs
#	mmtk/src/lib.rs
#	mmtk/src/scanning.rs
udesou added a commit to udesou/julia that referenced this pull request Feb 13, 2024
udesou pushed a commit to udesou/julia that referenced this pull request Aug 19, 2024
Testing:

- with a package error
```
(SimpleLooper) pkg> precompile
Precompiling all packages...
  ✗ SimpleLooper
  0 dependencies successfully precompiled in 2 seconds

ERROR: The following 1 direct dependency failed to precompile:

SimpleLooper 

Failed to precompile SimpleLooper [ff33fe5-d8e3-4cbd-8bd9-3d2408ff8cab] to "/Users/ian/.julia/compiled/v1.12/SimpleLooper/jl_PQArnH".
ERROR: LoadError: 
Stacktrace:
  [1] error()
    @ Base ./error.jl:53
```

- with interrupt
```
(SimpleLooper) pkg> precompile
Precompiling all packages...
^C Interrupted: Exiting precompilation...
  ◒ SimpleLooper
  1 dependency had output during precompilation:
┌ SimpleLooper
│  [57879] signal 2: Interrupt: 2
│  in expression starting at /Users/ian/Documents/GitHub/SimpleLooper.jl/src/SimpleLooper.jl:2
└  
```

- an internal error simulated in the same scope that
JuliaLang/Pkg.jl#3984 was failing to throw
from
 ```
  JULIA stdlib/release.image
Unhandled Task ERROR: 
Stacktrace:
 [1] error()
   @ Base ./error.jl:53
[2] (::Base.Precompilation.var"mmtk#27#65"{Bool, Bool, Vector{Task},
Dict{Tuple{Base.PkgId, Pair{Cmd, Base.CacheFlags}}, String},
Dict{Tuple{Base.PkgId, Pair{Cmd, Base.CacheFlags}}, String}, Base.Event,
Base.Event, ReentrantLock, Vector{Tuple{Base.PkgId, Pair{Cmd,
Base.CacheFlags}}}, Dict{Tuple{Base.PkgId, Pair{Cmd, Base.CacheFlags}},
String}, Vector{Tuple{Base.PkgId, Pair{Cmd, Base.CacheFlags}}}, Int64,
Vector{Base.PkgId}, Dict{Tuple{Base.PkgId, Pair{Cmd, Base.CacheFlags}},
Bool}, Dict{Tuple{Base.PkgId, Pair{Cmd, Base.CacheFlags}}, Base.Event},
Dict{Tuple{Base.PkgId, Pair{Cmd, Base.CacheFlags}}, Bool},
Vector{Base.PkgId}, Dict{Base.PkgId, String}, Dict{Tuple{Base.PkgId,
UInt128, String, String}, Bool},
Base.Precompilation.var"#color_string#38"{Bool}, Bool, Base.Semaphore,
Bool, String, Vector{String}, Vector{Base.PkgId}, Base.PkgId,
Base.CacheFlags, Cmd, Pair{Cmd, Base.CacheFlags}, Tuple{Base.PkgId,
Pair{Cmd, Base.CacheFlags}}})()
   @ Base.Precompilation ./precompilation.jl:819
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants