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.

Sign up for GitHub

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

Serialize GAP calls from within Julia. #505

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Serialize GAP calls from within Julia. #505

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fe0f25a
Serialize GAP calls from within Julia.
rbehrends Jul 25, 2020
9af4881
Remove old C-based synchronization code.
rbehrends Jul 31, 2020
7b639b1
Make global variables const instead.
rbehrends Jul 31, 2020
48ed5e3
Wrap entire initialize() function in @sync.
rbehrends Aug 1, 2020
8dfd851
Make locking logic task-based instead of thread-based.
rbehrends Aug 5, 2020
b4125ba
Fix precompilation issues arising from use of @sync macros.
rbehrends Aug 7, 2020
54b99fc
Add tests for thread safety.
rbehrends Aug 7, 2020
f6b28f7
Allow pinning GAP to a specific thread.
rbehrends Aug 14, 2020
6707eeb
Rename @sync -> @lock.
rbehrends Sep 8, 2020
d677f30
Document GAP.Sync.switch() method.
rbehrends Sep 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/JuliaInterface/Makefile.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# Makefile rules for the JuliaInterface package
#
KEXT_NAME = JuliaInterface
KEXT_SOURCES = src/JuliaInterface.c src/calls.c src/convert.c src/sync.c
KEXT_SOURCES = src/JuliaInterface.c src/calls.c src/convert.c

KEXT_CFLAGS =
KEXT_LDFLAGS =
Expand Down
29 changes: 0 additions & 29 deletions pkg/JuliaInterface/src/JuliaInterface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

#include "calls.h"
#include "convert.h"
#include "sync.h"

#include <src/julia_gc.h>

Expand Down Expand Up @@ -36,9 +35,7 @@ void handle_jl_exception(void)
jl_value_t * string_object =
jl_call1(JULIA_FUNC_take_inplace, JULIA_ERROR_IOBuffer);
string_object = jl_call1(JULIA_FUNC_String_constructor, string_object);
BEGIN_GAP_SYNC();
ErrorMayQuit(jl_string_data(string_object), 0, 0);
END_GAP_SYNC();
}

static jl_module_t * get_module(const char * name)
Expand All @@ -49,9 +46,7 @@ static jl_module_t * get_module(const char * name)
jl_value_t * module_value = jl_eval_string(name);
JULIAINTERFACE_EXCEPTION_HANDLER
if (!jl_is_module(module_value)) {
BEGIN_GAP_SYNC();
ErrorQuit("Not a module", 0, 0);
END_GAP_SYNC();
}
return (jl_module_t *)module_value;
}
Expand All @@ -64,9 +59,7 @@ Obj Func_JULIAINTERFACE_INTERNAL_INIT(Obj self)
JULIA_GAPFFE_type =
(jl_datatype_t *)jl_get_global(gap_module, jl_symbol("FFE"));
if (!JULIA_GAPFFE_type) {
BEGIN_GAP_SYNC();
ErrorMayQuit("Could not locate the GAP.FFE datatype", 0, 0);
END_GAP_SYNC();
}
return NULL;
}
Expand Down Expand Up @@ -98,15 +91,13 @@ int is_gapobj(jl_value_t * v)

static Obj Func_NewJuliaCFunc(Obj self, Obj julia_function_ptr, Obj args)
{
BEGIN_GAP_SYNC();
if (!IS_JULIA_OBJ(julia_function_ptr)) {
ErrorMayQuit("NewJuliaCFunc: <ptr> must be a Julia object", 0, 0);
}
RequirePlainList("NewJuliaCFunc", args);

jl_value_t * func_ptr = GET_JULIA_OBJ(julia_function_ptr);
void * ptr = jl_unbox_voidpointer(func_ptr);
END_GAP_SYNC();
return NewJuliaCFunc(ptr, args);
}

Expand Down Expand Up @@ -159,7 +150,6 @@ Obj NewJuliaObj(jl_value_t * v)
jl_function_t * get_function_from_obj_or_string(Obj func)
{
jl_function_t * f = NULL;
BEGIN_GAP_SYNC();
if (IS_JULIA_OBJ(func)) {
f = (jl_function_t *)GET_JULIA_OBJ(func);
}
Expand All @@ -173,7 +163,6 @@ jl_function_t * get_function_from_obj_or_string(Obj func)
}
else
ErrorMayQuit("argument is not a julia object or string", 0, 0);
END_GAP_SYNC();
return f;
}

Expand All @@ -195,7 +184,6 @@ static Obj Func_JuliaFunction(Obj self, Obj func)
*/
static Obj Func_JuliaFunctionByModule(Obj self, Obj funcName, Obj moduleName)
{
BEGIN_GAP_SYNC();
RequireStringRep("_JuliaFunctionByModule", funcName);
RequireStringRep("_JuliaFunctionByModule", moduleName);

Expand All @@ -205,7 +193,6 @@ static Obj Func_JuliaFunctionByModule(Obj self, Obj funcName, Obj moduleName)
jl_function_t * f = jl_get_function(m, CONST_CSTR_STRING(funcName));
if (f == 0)
ErrorMayQuit("Function is not defined in julia", 0, 0);
END_GAP_SYNC();
return NewJuliaFunc(f);
}

Expand All @@ -218,14 +205,12 @@ static Obj FuncIS_JULIA_FUNC(Obj self, Obj obj)
// Executes the string <string> in the current julia session.
static Obj FuncJuliaEvalString(Obj self, Obj string)
{
BEGIN_GAP_SYNC();
RequireStringRep("JuliaEvalString", string);

// It suffices to use JULIAINTERFACE_EXCEPTION_HANDLER here, as
// jl_eval_string is part of the jlapi, so don't have to be wrapped in
// JL_TRY/JL_CATCH.
jl_value_t * result = jl_eval_string(CONST_CSTR_STRING(string));
END_GAP_SYNC();
JULIAINTERFACE_EXCEPTION_HANDLER
return gap_julia(result);
}
Expand All @@ -234,26 +219,22 @@ static Obj FuncJuliaEvalString(Obj self, Obj string)
// :<name>.
static Obj FuncJuliaSymbol(Obj self, Obj name)
{
BEGIN_GAP_SYNC();
RequireStringRep("JuliaSymbol", name);

// jl_symbol never throws an exception and always returns a valid
// result, so no need for extra checks.
jl_sym_t * julia_symbol = jl_symbol(CONST_CSTR_STRING(name));
END_GAP_SYNC();
return NewJuliaObj((jl_value_t *)julia_symbol);
}

// Sets the value of the julia identifier <name> to the <val>.
// This function is for debugging purposes.
static Obj FuncJuliaSetVal(Obj self, Obj name, Obj val)
{
BEGIN_GAP_SYNC();
RequireStringRep("JuliaSetVal", name);

jl_value_t * julia_obj = julia_gap(val);
jl_sym_t * julia_symbol = jl_symbol(CONST_CSTR_STRING(name));
END_GAP_SYNC();
jl_set_global(jl_main_module, julia_symbol, julia_obj);
return 0;
}
Expand All @@ -262,11 +243,9 @@ static Obj FuncJuliaSetVal(Obj self, Obj name, Obj val)
// currently bound to the julia identifier <name>.
static Obj Func_JuliaGetGlobalVariable(Obj self, Obj name)
{
BEGIN_GAP_SYNC();
RequireStringRep("_JuliaGetGlobalVariable", name);

jl_sym_t * symbol = jl_symbol(CONST_CSTR_STRING(name));
END_GAP_SYNC();
if (!jl_boundp(jl_main_module, symbol)) {
return Fail;
}
Expand All @@ -278,7 +257,6 @@ static Obj Func_JuliaGetGlobalVariable(Obj self, Obj name)
// currently bound to the julia identifier <moduleName>.<name>.
static Obj Func_JuliaGetGlobalVariableByModule(Obj self, Obj name, Obj module)
{
BEGIN_GAP_SYNC();
RequireStringRep("_JuliaGetGlobalVariableByModule", name);

jl_module_t * m = 0;
Expand All @@ -296,7 +274,6 @@ static Obj Func_JuliaGetGlobalVariableByModule(Obj self, Obj name, Obj module)
0, 0);
}
jl_sym_t * symbol = jl_symbol(CONST_CSTR_STRING(name));
END_GAP_SYNC();
if (!jl_boundp(m, symbol)) {
return Fail;
}
Expand All @@ -309,7 +286,6 @@ static Obj Func_JuliaGetGlobalVariableByModule(Obj self, Obj name, Obj module)
// <super_object> must be a julia object GAP object, and <name> a string.
static Obj FuncJuliaGetFieldOfObject(Obj self, Obj super_obj, Obj field_name)
{
BEGIN_GAP_SYNC();
if (!IS_JULIA_OBJ(super_obj)) {
ErrorMayQuit(
"JuliaGetFieldOfObject: <super_obj> must be a Julia object", 0,
Expand All @@ -324,7 +300,6 @@ static Obj FuncJuliaGetFieldOfObject(Obj self, Obj super_obj, Obj field_name)
// JL_TRY/JL_CATCH.
jl_value_t * field_value =
jl_get_field(extracted_superobj, CONST_CSTR_STRING(field_name));
END_GAP_SYNC();
JULIAINTERFACE_EXCEPTION_HANDLER
return gap_julia(field_value);
}
Expand All @@ -335,7 +310,6 @@ static Obj FuncSTREAM_CALL(Obj self, Obj stream, Obj func, Obj obj)
{
syJmp_buf readJmpError;

BEGIN_GAP_SYNC();

if (CALL_1ARGS(IsOutputStream, stream) != True) {
ErrorQuit("STREAM_CALL: <outstream> must be an output stream", 0, 0);
Expand All @@ -351,7 +325,6 @@ static Obj FuncSTREAM_CALL(Obj self, Obj stream, Obj func, Obj obj)
if (!CloseOutput()) {
ErrorQuit("STREAM_CALL: cannot close output", 0, 0);
}
END_GAP_SYNC();

return 0;
}
Expand Down Expand Up @@ -396,8 +369,6 @@ static Int InitKernel(StructInitInfo * module)
jl_options.can_inline = 0;
}

InitGapSync();

// init filters and functions
InitHdlrFuncsFromTable(GVarFuncs);

Expand Down
7 changes: 0 additions & 7 deletions pkg/JuliaInterface/src/calls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#include "calls.h"
#include "convert.h"
#include "sync.h"
#include "JuliaInterface.h"


Expand All @@ -23,7 +22,6 @@ jl_value_t * call_gap_func(Obj func, jl_value_t * args)

size_t len = jl_nfields(args);
Obj return_value = NULL;
BEGIN_GAP_SYNC();
if (IS_FUNC(func) && len <= 6) {
switch (len) {
case 0:
Expand Down Expand Up @@ -73,7 +71,6 @@ jl_value_t * call_gap_func(Obj func, jl_value_t * args)
}
return_value = CallFuncList(func, arg_list);
}
END_GAP_SYNC();
if (return_value == NULL) {
return jl_nothing;
}
Expand Down Expand Up @@ -191,7 +188,6 @@ static Obj DoCallJuliaFuncXArg(Obj func, Obj args)
//
Obj NewJuliaFunc(jl_function_t * function)
{
BEGIN_GAP_SYNC();
Obj name = MakeImmString(jl_symbol_name(jl_gf_name(function)));
Obj func = NewFunctionT(T_FUNCTION, sizeof(JuliaFuncBag), name, -1,
ArgStringToList("arg"), 0);
Expand All @@ -217,7 +213,6 @@ Obj NewJuliaFunc(jl_function_t * function)
SET_BODY_FUNC(func, body);
CHANGED_BAG(body);
CHANGED_BAG(func);
END_GAP_SYNC();

return func;
}
Expand Down Expand Up @@ -310,7 +305,6 @@ Obj NewJuliaCFunc(void * function, Obj args)
{
ObjFunc handler;

BEGIN_GAP_SYNC();
switch (LEN_PLIST(args)) {
case 0:
handler = DoCallJuliaCFunc0Arg;
Expand Down Expand Up @@ -345,7 +339,6 @@ Obj NewJuliaCFunc(void * function, Obj args)
// store it as a valid julia obj (i.e., void ptr).
((JuliaFuncBag *)ADDR_OBJ(func))->juliaFunc =
NewJuliaObj(jl_box_voidpointer(function));
END_GAP_SYNC();

return func;
}
3 changes: 0 additions & 3 deletions pkg/JuliaInterface/src/convert.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#include "convert.h"

#include "calls.h"
#include "sync.h"
#include "JuliaInterface.h"

// This function is used by GAP.jl
Expand Down Expand Up @@ -29,7 +28,6 @@ jl_value_t * julia_gap(Obj obj)
return jl_false;
}
jl_value_t * result = (jl_value_t *)obj;
BEGIN_GAP_SYNC();
if (TNUM_OBJ(obj) >= FIRST_EXTERNAL_TNUM &&
CALL_1ARGS(JULIAINTERFACE_IsJuliaWrapper, obj) == True) {
obj = CALL_1ARGS(JULIAINTERFACE_JuliaPointer, obj);
Expand All @@ -49,7 +47,6 @@ jl_value_t * julia_gap(Obj obj)
(Int)TNAM_OBJ(obj), 0);
}
}
END_GAP_SYNC();
return result;
}

Expand Down
26 changes: 0 additions & 26 deletions pkg/JuliaInterface/src/sync.c
View file
Open in desktop

This file was deleted.

20 changes: 0 additions & 20 deletions pkg/JuliaInterface/src/sync.h
View file
Open in desktop

This file was deleted.

11 changes: 6 additions & 5 deletions src/GAP.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ if !isfile(deps_jl)
Pkg.build("GAP")
end
include(deps_jl)
include("sync.jl")


"""
Expand Down Expand Up @@ -290,7 +291,7 @@ function initialize(argv::Array{String,1})
end

function finalize()
ccall((:GAP_finalize, "libgap"), Cvoid, ())
@sync ccall((:GAP_finalize, "libgap"), Cvoid, ())
end

function run_it()
Expand All @@ -313,7 +314,7 @@ function run_it()
# Do not show the main GAP banner by default.
push!(cmdline_options, "-b")
end
initialize(cmdline_options)
@sync initialize(cmdline_options)

if !show_banner
# Leave it to GAP's `LoadPackage` whether package banners are shown.
Expand Down Expand Up @@ -383,10 +384,10 @@ function prompt()
# save the current SIGINT handler
# HACK: the hardcoded value for SIG_DFL is not portable, revise this
# install GAP's SIGINT handler
old_sigint = ccall(:signal, Ptr{Cvoid}, (Cint, Ptr{Cvoid}), Base.SIGINT, C_NULL)
old_sigint = @sync ccall(:signal, Ptr{Cvoid}, (Cint, Ptr{Cvoid}), Base.SIGINT, C_NULL)
Copy link
Member

Choose a reason for hiding this comment

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

Does that make sense for a call into the C library? Anyway, perhaps we can just restrict GAP.prompt() to the main thread or so somehow? If we ever end up racing this function from multiple threads, all hell will break loose anyway...


# install GAP's SIGINT handler
ccall(:SyInstallAnswerIntr, Cvoid, ())
@sync ccall(:SyInstallAnswerIntr, Cvoid, ())

# restore GAP's error output
disable_error_handler = true
Expand All @@ -404,7 +405,7 @@ function prompt()
Globals.BreakOnError = false

# restore signal handler
ccall(:signal, Ptr{Cvoid}, (Cint, Ptr{Cvoid}), Base.SIGINT, old_sigint)
@sync ccall(:signal, Ptr{Cvoid}, (Cint, Ptr{Cvoid}), Base.SIGINT, old_sigint)

# restore GAP error handler
disable_error_handler = false
Expand Down
Loading