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

Jump to bottom

Release the GIL for geometry-creating operations #156

Merged
merged 21 commits into from Jul 3, 2020
Merged

Release the GIL for geometry-creating operations #156

Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
131f124
WIP Releasing GIL on Y_Y operations
caspervdw Jun 1, 2020
adccf86
Finish impl for Y_Y
caspervdw Jun 1, 2020
dda1d55
Release GIL in Yd_Y_func and Yi_Y_func
caspervdw Jun 4, 2020
9027dda
Add decorators
caspervdw Jun 4, 2020
2c600b6
Buffer
caspervdw Jun 4, 2020
622d5e8
Simplify geom_arr==NULL path
caspervdw Jun 6, 2020
2212d4b
set operations (breaks reduce)
caspervdw Jun 6, 2020
b65a415
Working draft version for YY_Y without GIL
caspervdw Jun 6, 2020
91a2de3
Revert "Working draft version for YY_Y without GIL"
caspervdw Jun 7, 2020
a7f6dad
Take 2; simpler approach
caspervdw Jun 7, 2020
1abb0c0
Revert "Take 2; simpler approach"
caspervdw Jun 7, 2020
63fd38d
Separate Y_YY_reduce function
caspervdw Jun 7, 2020
9538e24
Delaunay triangles
caspervdw Jun 7, 2020
84c833b
Voronoi polygons
caspervdw Jun 7, 2020
5245090
Fix doctests and rearrange Y_YY_func_reduce
caspervdw Jun 7, 2020
84388d5
PR Changes
caspervdw Jun 17, 2020
97e5b3c
Clean commented out code
caspervdw Jun 17, 2020
9a3d560
Comments and init pointers with NULL
caspervdw Jun 18, 2020
c7f4b7a
Docstrings
caspervdw Jun 21, 2020
25fe256
Update src/ufuncs.c
caspervdw Jul 2, 2020
db9fd2e
Make snap release the GIL
caspervdw Jul 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
120 changes: 80 additions & 40 deletions src/ufuncs.c
View file
Open in desktop
Expand Up @@ -267,14 +267,14 @@ static void Y_Y_func(char **args, npy_intp *dimensions,

if (geom_arr != NULL) {
UNARY_LOOP {
/* get the geometry: return on error */
// get the geometry: return on error
if (!get_geom(*(GeometryObject **)ip1, &in1)) {
errstate = PGERR_NOT_A_GEOMETRY;
destroy_geom_arr(ctx, geom_arr, i - 1);
break;
}
if (in1 == NULL) {
/* in case of a missing value: return NULL (None) */
// in case of a missing value: return NULL (None)
geom_arr[i] = NULL;
} else {
geom_arr[i] = func(ctx, in1);
Expand All @@ -288,9 +288,8 @@ static void Y_Y_func(char **args, npy_intp *dimensions,
}
}
}
} else {
errstate = PGERR_NO_MALLOC;
}


GEOS_FINISH_THREADS;

Expand Down Expand Up @@ -340,25 +339,47 @@ static void Yd_Y_func(char **args, npy_intp *dimensions,
npy_intp *steps, void *data)
{
FuncGEOS_Yd_Y *func = (FuncGEOS_Yd_Y *)data;
GEOSGeometry *in1, *ret_ptr;
GEOSGeometry *in1;

GEOS_INIT;
// allocate a temporary array to store output GEOSGeometry objects
GEOSGeometry **geom_arr = malloc(sizeof(void *) * dimensions[0]);

BINARY_LOOP {
/* get the geometry: return on error */
if (!get_geom(*(GeometryObject **)ip1, &in1)) { errstate = PGERR_NOT_A_GEOMETRY; goto finish; }
double in2 = *(double *)ip2;
if ((in1 == NULL) | (npy_isnan(in2))) {
ret_ptr = NULL;
} else {
ret_ptr = func(ctx, in1, in2);
if (ret_ptr == NULL) { errstate = PGERR_GEOS_EXCEPTION; goto finish; }
GEOS_INIT_THREADS;

if (geom_arr != NULL) {
BINARY_LOOP {
// get the geometry: return on error
if (!get_geom(*(GeometryObject **)ip1, &in1)) {
errstate = PGERR_NOT_A_GEOMETRY;
destroy_geom_arr(ctx, geom_arr, i - 1);
break;
}
double in2 = *(double *)ip2;
if ((in1 == NULL) | (npy_isnan(in2))) {
// in case of a missing value: return NULL (None)
geom_arr[i] = NULL;
} else {
geom_arr[i] = func(ctx, in1, in2);
if (geom_arr[i] == NULL) {
errstate = PGERR_GEOS_EXCEPTION;
destroy_geom_arr(ctx, geom_arr, i - 1);
break;
}
}
}
OUTPUT_Y;
} else {
errstate = PGERR_NO_MALLOC;
Copy link
Contributor

Choose a reason for hiding this comment

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

@caspervdw what do you think about moving this up below line 345, with a return immediately after? Something like:

GEOSGeometry **geom_arr = malloc(sizeof(void *) * dimensions[0]);
if (geom_arr == NULL) {
    errstate = PGERR_NO_MALLOC;
    return
}

Then the "happy path" becomes simpler:

GEOS_INIT_THREADS;
BINARY_LOOP { ... }
GEOS_FINISH_THREADS;

if (errstate == PGERR_SUCCESS) {
    geom_arr_to_npy(geom_arr, args[2], steps[2], dimensions[0]);
}
free(geom_arr);

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! The errstate variable doesn't exist outside of the GEOS_INIT and GEOS_FINISH, but I'll just set the python errorstate directly and return instead.

}

finish:
GEOS_FINISH;
GEOS_FINISH_THREADS;

// fill the numpy array with PyObjects while holding the GIL
if (geom_arr != NULL) {
if (errstate == PGERR_SUCCESS) {
geom_arr_to_npy(geom_arr, args[2], steps[2], dimensions[0]);
}
free(geom_arr);
}
}
static PyUFuncGenericFunction Yd_Y_funcs[1] = {&Yd_Y_func};

Expand Down Expand Up @@ -453,31 +474,50 @@ static void Yi_Y_func(char **args, npy_intp *dimensions,
npy_intp *steps, void *data)
{
FuncGEOS_Yi_Y *func = (FuncGEOS_Yi_Y *)data;
GEOSGeometry *in1, *ret_ptr;
GEOSGeometry *in1;

GEOS_INIT;
// allocate a temporary array to store output GEOSGeometry objects
GEOSGeometry **geom_arr = malloc(sizeof(void *) * dimensions[0]);

BINARY_LOOP {
/* get the geometry: return on error */
if (!get_geom(*(GeometryObject **)ip1, &in1)) { errstate = PGERR_NOT_A_GEOMETRY; goto finish; }
int in2 = *(int *) ip2;
if (in1 == NULL) {
ret_ptr = NULL;
} else {
ret_ptr = func(ctx, in1, in2);
// NULL means: exception, but for some functions it may also indicate a
// "missing value" (None) (GetPointN, GetInteriorRingN, GetGeometryN)
// So: check the last_error before setting error state
if ((ret_ptr == NULL) && (last_error[0] != 0)) {
errstate = PGERR_GEOS_EXCEPTION;
goto finish;
GEOS_INIT_THREADS;

if (geom_arr != NULL) {
BINARY_LOOP {
// get the geometry: return on error
if (!get_geom(*(GeometryObject **)ip1, &in1)) {
errstate = PGERR_NOT_A_GEOMETRY;
destroy_geom_arr(ctx, geom_arr, i - 1);
break;
}
int in2 = *(int *) ip2;
if (in1 == NULL) {
// in case of a missing value: return NULL (None)
geom_arr[i] = NULL;
} else {
geom_arr[i] = func(ctx, in1, in2);
// NULL means: exception, but for some functions it may also indicate a
// "missing value" (None) (GetPointN, GetInteriorRingN, GetGeometryN)
// So: check the last_error before setting error state
if ((geom_arr[i] == NULL) && (last_error[0] != 0)) {
errstate = PGERR_GEOS_EXCEPTION;
destroy_geom_arr(ctx, geom_arr, i - 1);
break;
}
}
}
OUTPUT_Y;
} else {
errstate = PGERR_NO_MALLOC;
}

finish:
GEOS_FINISH;
GEOS_FINISH_THREADS;

// fill the numpy array with PyObjects while holding the GIL
if (geom_arr != NULL) {
if (errstate == PGERR_SUCCESS) {
geom_arr_to_npy(geom_arr, args[2], steps[2], dimensions[0]);
}
free(geom_arr);
}
}
static PyUFuncGenericFunction Yi_Y_funcs[1] = {&Yi_Y_func};

Expand All @@ -497,14 +537,14 @@ static void YY_Y_func(char **args, npy_intp *dimensions,

GEOS_INIT;

BINARY_LOOP {
BINARY_LOOP {
/* get the geometries: return on error */
if (!get_geom(*(GeometryObject **)ip1, &in1)) { errstate = PGERR_NOT_A_GEOMETRY; goto finish; }
if (!get_geom(*(GeometryObject **)ip2, &in2)) { errstate = PGERR_NOT_A_GEOMETRY; goto finish; }
if ((in1 == NULL) | (in2 == NULL)) {
if ((in1 == NULL) | (in2 == NULL)) {
/* in case of a missing value: return NULL (None) */
ret_ptr = NULL;
} else {
} else {
ret_ptr = func(ctx, in1, in2);
if (ret_ptr == NULL) { errstate = PGERR_GEOS_EXCEPTION; goto finish; }
}
Expand Down