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

Make the Julia GC threadsafe when used from GAP.jl #4071

Merged
merged 1 commit into from
Jul 14, 2020

Conversation

rbehrends
Copy link
Contributor

When running from multi-threaded Julia, we have to consider the case
that GAP can be called from more than one thread. In those cases, the
variable JuliaTLS may be incorrect. We therefore update it in NewBag(),
where it is depended upon for allocations. It had already been set when
a GC was called from a separate thread.

In order to avoid unnecessary overhead in the single-threaded case, we
do not change JuliaTLS for allocations when there is only one thread, as
determining the current thread-local state pointer can be somewhat
costly (which is why we cache it in JuliaTLS in the first place).

When running from multi-threaded Julia, we have to consider the case
that GAP can be called from more than one thread. In those cases, the
variable JuliaTLS may be incorrect. We therefore update it in NewBag(),
where it is depended upon for allocations. It had already been set when
a GC was called from a separate thread.

In order to avoid unnecessary overhead in the single-threaded case, we
do not change JuliaTLS for allocations when there is only one thread, as
determining the current thread-local state pointer can be somewhat
costly (which is why we cache it in JuliaTLS in the first place).
@coveralls
Copy link

coveralls commented Jul 10, 2020

Coverage Status

Coverage decreased (-3.0e-05%) to 84.882% when pulling 4468a50 on rbehrends:gap-jl-threadsafe into 2d0fabb on gap-system:master.

@fingolfin fingolfin added topic: julia Julia GC integration and related matters topic: kernel labels Jul 10, 2020
src/julia_gc.c Outdated
@@ -199,11 +199,17 @@ static jl_datatype_t * datatype_largebag;
static UInt StackAlignBags;
static Bag * GapStackBottom;
static jl_ptls_t JuliaTLS, SaveTLS;
static int is_threaded;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
static int is_threaded;
static BOOL is_threaded;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it.

@fingolfin fingolfin merged commit b4c5176 into gap-system:master Jul 14, 2020
@fingolfin fingolfin deleted the gap-jl-threadsafe branch July 14, 2020 10:45
@fingolfin
Copy link
Member

Backported to stable-4.11 in 27ebdf9

@PaulaHaehndel PaulaHaehndel self-assigned this Feb 16, 2021
@PaulaHaehndel PaulaHaehndel added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Feb 16, 2021
@PaulaHaehndel PaulaHaehndel removed their assignment Feb 16, 2021
@PaulaHaehndel PaulaHaehndel changed the title Make the Julia GC threadsafe when used from GAP.jl. Make the Julia GC threadsafe when used from GAP.jl Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-4.11-DONE release notes: added PRs introducing changes that have since been mentioned in the release notes topic: julia Julia GC integration and related matters topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants