Skip to content

Commit

Permalink
c-ext: conditionally enable features relying on pool APIs
Browse files Browse the repository at this point in the history
The pool APIs are not exported from the library nor are they present
in public headers.

This commit changes behavior of the C backend to conditionally enable
features relying on these symbols. The practical effect is that these
features will only be available if building against the single file /
bundled libzstd and will no longer be available when building against a
system libzstd.

This effectively closes #106.

A side-effect of this change is that --system-zstd likely regressed
due to inability to find zstd headers. We'll need to add a setup.py
argument to plumb the header search path through. But this was a
pre-existing condition.
  • Loading branch information
indygreg committed Dec 31, 2020
1 parent d0aff43 commit 2752c49
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 223 deletions.
4 changes: 4 additions & 0 deletions c-ext/backend_c.c
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ void zstd_module_init(PyObject *m) {

Py_DECREF(feature);

#ifdef HAVE_ZSTD_POOL_APIS
feature = PyUnicode_FromString("multi_compress_to_buffer");
if (NULL == feature) {
PyErr_SetString(PyExc_ImportError, "could not create feature string");
Expand All @@ -193,7 +194,9 @@ void zstd_module_init(PyObject *m) {
}

Py_DECREF(feature);
#endif

#ifdef HAVE_ZSTD_POOL_APIS
feature = PyUnicode_FromString("multi_decompress_to_buffer");
if (NULL == feature) {
PyErr_SetString(PyExc_ImportError, "could not create feature string");
Expand All @@ -205,6 +208,7 @@ void zstd_module_init(PyObject *m) {
}

Py_DECREF(feature);
#endif

if (PyObject_SetAttrString(m, "backend_features", features) == -1) {
return;
Expand Down
16 changes: 11 additions & 5 deletions c-ext/compressor.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@

#include "python-zstandard.h"

/* TODO pool.h is a private header and we shouldn't rely on it. */
#ifndef ZSTD_SINGLE_FILE
#include "pool.h"
#endif

extern PyObject *ZstdError;

int setup_cctx(ZstdCompressor *compressor) {
Expand Down Expand Up @@ -822,6 +817,7 @@ typedef struct {
Py_ssize_t errorOffset;
} CompressorWorkerState;

#ifdef HAVE_ZSTD_POOL_APIS
static void compress_worker(CompressorWorkerState *state) {
Py_ssize_t inputOffset = state->startOffset;
Py_ssize_t remainingItems = state->endOffset - state->startOffset + 1;
Expand Down Expand Up @@ -1043,6 +1039,11 @@ static void compress_worker(CompressorWorkerState *state) {
destBuffer->destSize = destOffset;
}
}
#endif

/* We can only use the pool.h APIs if we provide the full library,
as these are private APIs. */
#ifdef HAVE_ZSTD_POOL_APIS

ZstdBufferWithSegmentsCollection *
compress_from_datasources(ZstdCompressor *compressor, DataSources *sources,
Expand Down Expand Up @@ -1298,7 +1299,9 @@ compress_from_datasources(ZstdCompressor *compressor, DataSources *sources,

return result;
}
#endif

#ifdef HAVE_ZSTD_POOL_APIS
static ZstdBufferWithSegmentsCollection *
ZstdCompressor_multi_compress_to_buffer(ZstdCompressor *self, PyObject *args,
PyObject *kwargs) {
Expand Down Expand Up @@ -1463,6 +1466,7 @@ ZstdCompressor_multi_compress_to_buffer(ZstdCompressor *self, PyObject *args,

return result;
}
#endif

static PyMethodDef ZstdCompressor_methods[] = {
{"chunker", (PyCFunction)ZstdCompressor_chunker,
Expand All @@ -1479,9 +1483,11 @@ static PyMethodDef ZstdCompressor_methods[] = {
METH_VARARGS | METH_KEYWORDS, NULL},
{"read_to_iter", (PyCFunction)ZstdCompressor_read_to_iter,
METH_VARARGS | METH_KEYWORDS, NULL},
#ifdef HAVE_ZSTD_POOL_APIS
{"multi_compress_to_buffer",
(PyCFunction)ZstdCompressor_multi_compress_to_buffer,
METH_VARARGS | METH_KEYWORDS, NULL},
#endif
{"memory_size", (PyCFunction)ZstdCompressor_memory_size, METH_NOARGS, NULL},
{"frame_progression", (PyCFunction)ZstdCompressor_frame_progression,
METH_NOARGS, NULL},
Expand Down
13 changes: 8 additions & 5 deletions c-ext/decompressor.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@

#include "python-zstandard.h"

/* TODO pool.h is a private header and we shouldn't rely on it. */
#ifndef ZSTD_SINGLE_FILE
#include "pool.h"
#endif

extern PyObject *ZstdError;

/**
Expand Down Expand Up @@ -903,6 +898,7 @@ typedef struct {
size_t zresult;
} DecompressorWorkerState;

#ifdef HAVE_ZSTD_POOL_APIS
static void decompress_worker(DecompressorWorkerState *state) {
size_t allocationSize;
DecompressorDestBuffer *destBuffer;
Expand Down Expand Up @@ -1144,7 +1140,9 @@ static void decompress_worker(DecompressorWorkerState *state) {
destBuffer->destSize = destOffset;
}
}
#endif

#ifdef HAVE_ZSTD_POOL_APIS
ZstdBufferWithSegmentsCollection *
decompress_from_framesources(ZstdDecompressor *decompressor,
FrameSources *frames, Py_ssize_t threadCount) {
Expand Down Expand Up @@ -1418,7 +1416,9 @@ decompress_from_framesources(ZstdDecompressor *decompressor,

return result;
}
#endif

#ifdef HAVE_ZSTD_POOL_APIS
static ZstdBufferWithSegmentsCollection *
Decompressor_multi_decompress_to_buffer(ZstdDecompressor *self, PyObject *args,
PyObject *kwargs) {
Expand Down Expand Up @@ -1672,6 +1672,7 @@ Decompressor_multi_decompress_to_buffer(ZstdDecompressor *self, PyObject *args,

return result;
}
#endif

static PyMethodDef Decompressor_methods[] = {
{"copy_stream", (PyCFunction)Decompressor_copy_stream,
Expand All @@ -1689,9 +1690,11 @@ static PyMethodDef Decompressor_methods[] = {
{"decompress_content_dict_chain",
(PyCFunction)Decompressor_decompress_content_dict_chain,
METH_VARARGS | METH_KEYWORDS, NULL},
#ifdef HAVE_ZSTD_POOL_APIS
{"multi_decompress_to_buffer",
(PyCFunction)Decompressor_multi_decompress_to_buffer,
METH_VARARGS | METH_KEYWORDS, NULL},
#endif
{"memory_size", (PyCFunction)Decompressor_memory_size, METH_NOARGS, NULL},
{NULL, NULL}};

Expand Down
5 changes: 5 additions & 0 deletions c-ext/python-zstandard.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@

#ifdef ZSTD_SINGLE_FILE
#include <zstdlib.c>

/* We use private APIs from pool.h. We can't rely on availability
of this header or symbols when linking against the system libzstd.
But we know it works when using the bundled single file library. */
#define HAVE_ZSTD_POOL_APIS
#else
#include <zdict.h>
#include <zstd.h>
Expand Down
9 changes: 9 additions & 0 deletions docs/news.rst
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ Other Actions Not Blocking Release
0.16.0 (not yet released)
=========================

Backwards Compatibility Notes
-----------------------------

* ``ZstdCompressor.multi_compress_to_buffer()`` and
``ZstdDecompressor.multi_decompress_to_buffer()`` are no longer
available when linking against a system zstd library. These
experimental features are only available when building against the
bundled single file zstd C source file distribution. (#106)

0.15.1 (released 2020-12-31)
============================

Expand Down
19 changes: 1 addition & 18 deletions setup_zstd.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@
"c-ext/backend_c.c",
]

zstd_includes = [
"zstd",
"zstd/common",
"zstd/dictBuilder",
]


def get_c_extension(
support_legacy=False,
Expand Down Expand Up @@ -59,18 +53,7 @@ def get_c_extension(
sources = sorted(set([os.path.join(actual_root, p) for p in ext_sources]))
local_include_dirs = [os.path.join(actual_root, d) for d in ext_includes]

if system_zstd:
# TODO remove this once pool.h dependency goes away.
#
# This effectively causes system zstd mode to pull in our
# local headers instead of the system's. Then we link with the
# system library. This is super sketchy and could result in link
# time errors due to symbol mismatch or even run-time errors if
# APIs behave differently.
local_include_dirs.extend(
[os.path.join(actual_root, d) for d in zstd_includes]
)
else:
if not system_zstd:
local_include_dirs.append(os.path.join(actual_root, "zstd"))

depends = sorted(glob.glob(os.path.join(actual_root, "c-ext", "*")))
Expand Down
84 changes: 0 additions & 84 deletions zstd/common/pool.h

This file was deleted.

Loading

0 comments on commit 2752c49

Please sign in to comment.