From c2ed4f2f3f34639cb878f7b4d9d97b7e68c1a458 Mon Sep 17 00:00:00 2001 From: Christian Klein <167265+cklein@users.noreply.github.com> Date: Tue, 13 Apr 2021 18:28:25 +0200 Subject: [PATCH] Changed UnicodeBuilders to operate on a c string instead of a list of Python strings --- hpy/debug/src/autogen_debug_ctx_init.h | 2 +- hpy/debug/src/autogen_debug_wrappers.c | 4 +- .../common/runtime/ctx_unicodebuilder.h | 3 +- hpy/devel/include/cpython/hpy.h | 4 +- hpy/devel/include/universal/autogen_ctx.h | 2 +- .../include/universal/autogen_trampolines.h | 4 +- hpy/devel/src/runtime/ctx_unicodebuilder.c | 62 +++++++++---------- hpy/tools/autogen/public_api.h | 2 +- test/test_hpyunicodebuilder.py | 40 ++---------- 9 files changed, 44 insertions(+), 79 deletions(-) diff --git a/hpy/debug/src/autogen_debug_ctx_init.h b/hpy/debug/src/autogen_debug_ctx_init.h index cc1acd862..11f991a14 100644 --- a/hpy/debug/src/autogen_debug_ctx_init.h +++ b/hpy/debug/src/autogen_debug_ctx_init.h @@ -132,7 +132,7 @@ void debug_ctx_ListBuilder_Set(HPyContext *dctx, HPyListBuilder builder, HPy_ssi DHPy debug_ctx_ListBuilder_Build(HPyContext *dctx, HPyListBuilder builder); void debug_ctx_ListBuilder_Cancel(HPyContext *dctx, HPyListBuilder builder); HPyUnicodeBuilder debug_ctx_UnicodeBuilder_New(HPyContext *dctx, HPy_ssize_t size); -int debug_ctx_UnicodeBuilder_Add(HPyContext *dctx, HPyUnicodeBuilder builder, DHPy h_item); +int debug_ctx_UnicodeBuilder_Add(HPyContext *dctx, HPyUnicodeBuilder builder, const char *item); DHPy debug_ctx_UnicodeBuilder_Build(HPyContext *dctx, HPyUnicodeBuilder builder); void debug_ctx_UnicodeBuilder_Cancel(HPyContext *dctx, HPyUnicodeBuilder builder); HPyTupleBuilder debug_ctx_TupleBuilder_New(HPyContext *dctx, HPy_ssize_t initial_size); diff --git a/hpy/debug/src/autogen_debug_wrappers.c b/hpy/debug/src/autogen_debug_wrappers.c index f0affd025..486e16295 100644 --- a/hpy/debug/src/autogen_debug_wrappers.c +++ b/hpy/debug/src/autogen_debug_wrappers.c @@ -597,9 +597,9 @@ HPyUnicodeBuilder debug_ctx_UnicodeBuilder_New(HPyContext *dctx, HPy_ssize_t siz return HPyUnicodeBuilder_New(get_info(dctx)->uctx, size); } -int debug_ctx_UnicodeBuilder_Add(HPyContext *dctx, HPyUnicodeBuilder builder, DHPy h_item) +int debug_ctx_UnicodeBuilder_Add(HPyContext *dctx, HPyUnicodeBuilder builder, const char *item) { - return HPyUnicodeBuilder_Add(get_info(dctx)->uctx, builder, DHPy_unwrap(dctx, h_item)); + return HPyUnicodeBuilder_Add(get_info(dctx)->uctx, builder, item); } DHPy debug_ctx_UnicodeBuilder_Build(HPyContext *dctx, HPyUnicodeBuilder builder) diff --git a/hpy/devel/include/common/runtime/ctx_unicodebuilder.h b/hpy/devel/include/common/runtime/ctx_unicodebuilder.h index ce7ad975a..5b7924369 100644 --- a/hpy/devel/include/common/runtime/ctx_unicodebuilder.h +++ b/hpy/devel/include/common/runtime/ctx_unicodebuilder.h @@ -7,8 +7,7 @@ _HPy_HIDDEN HPyUnicodeBuilder ctx_UnicodeBuilder_New(HPyContext *ctx, HPy_ssize_t capacity); -_HPy_HIDDEN int ctx_UnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, - HPy h_item); +_HPy_HIDDEN int ctx_UnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, const char *item); _HPy_HIDDEN HPy ctx_UnicodeBuilder_Build(HPyContext *ctx, HPyUnicodeBuilder builder); _HPy_HIDDEN void ctx_UnicodeBuilder_Cancel(HPyContext *ctx, HPyUnicodeBuilder builder); diff --git a/hpy/devel/include/cpython/hpy.h b/hpy/devel/include/cpython/hpy.h index b60305026..27e0c6b92 100644 --- a/hpy/devel/include/cpython/hpy.h +++ b/hpy/devel/include/cpython/hpy.h @@ -442,9 +442,9 @@ HPyUnicodeBuilder_New(HPyContext *ctx, HPy_ssize_t size) } HPyAPI_FUNC(int) -HPyUnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, HPy h_item) +HPyUnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, const char *item) { - return ctx_UnicodeBuilder_Add(ctx, builder, h_item); + return ctx_UnicodeBuilder_Add(ctx, builder, item); } HPyAPI_FUNC(HPy) diff --git a/hpy/devel/include/universal/autogen_ctx.h b/hpy/devel/include/universal/autogen_ctx.h index 22aa45996..a90643fd7 100644 --- a/hpy/devel/include/universal/autogen_ctx.h +++ b/hpy/devel/include/universal/autogen_ctx.h @@ -211,7 +211,7 @@ struct _HPyContext_s { HPy (*ctx_ListBuilder_Build)(HPyContext *ctx, HPyListBuilder builder); void (*ctx_ListBuilder_Cancel)(HPyContext *ctx, HPyListBuilder builder); HPyUnicodeBuilder (*ctx_UnicodeBuilder_New)(HPyContext *ctx, HPy_ssize_t size); - int (*ctx_UnicodeBuilder_Add)(HPyContext *ctx, HPyUnicodeBuilder builder, HPy h_item); + int (*ctx_UnicodeBuilder_Add)(HPyContext *ctx, HPyUnicodeBuilder builder, const char *item); HPy (*ctx_UnicodeBuilder_Build)(HPyContext *ctx, HPyUnicodeBuilder builder); void (*ctx_UnicodeBuilder_Cancel)(HPyContext *ctx, HPyUnicodeBuilder builder); HPyTupleBuilder (*ctx_TupleBuilder_New)(HPyContext *ctx, HPy_ssize_t initial_size); diff --git a/hpy/devel/include/universal/autogen_trampolines.h b/hpy/devel/include/universal/autogen_trampolines.h index b5999941c..c52b785a5 100644 --- a/hpy/devel/include/universal/autogen_trampolines.h +++ b/hpy/devel/include/universal/autogen_trampolines.h @@ -490,8 +490,8 @@ static inline HPyUnicodeBuilder HPyUnicodeBuilder_New(HPyContext *ctx, HPy_ssize return ctx->ctx_UnicodeBuilder_New ( ctx, size ); } -static inline int HPyUnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, HPy h_item) { - return ctx->ctx_UnicodeBuilder_Add ( ctx, builder, h_item ); +static inline int HPyUnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, const char *item) { + return ctx->ctx_UnicodeBuilder_Add ( ctx, builder, item ); } static inline HPy HPyUnicodeBuilder_Build(HPyContext *ctx, HPyUnicodeBuilder builder) { diff --git a/hpy/devel/src/runtime/ctx_unicodebuilder.c b/hpy/devel/src/runtime/ctx_unicodebuilder.c index 5716b3652..ea96f3ba7 100644 --- a/hpy/devel/src/runtime/ctx_unicodebuilder.c +++ b/hpy/devel/src/runtime/ctx_unicodebuilder.c @@ -1,4 +1,5 @@ #include +#include #include #include "hpy.h" @@ -7,12 +8,12 @@ # include "handles.h" #endif -static const Py_ssize_t HPYUNICODEBUILDER_INITIAL_CAPACITY = 5; +static const Py_ssize_t HPYUNICODEBUILDER_INITIAL_CAPACITY = 1024; typedef struct { Py_ssize_t capacity; // allocated handles - Py_ssize_t length; // used handles - PyObject *list; + Py_ssize_t length; + char *buf; } _PyUnicodeBuilder_s; #ifdef HPY_UNIVERSAL_ABI @@ -36,9 +37,10 @@ ctx_UnicodeBuilder_New(HPyContext *ctx, HPy_ssize_t capacity) { _PyUnicodeBuilder_s *bp; if (capacity == 0) { + // TODO: default value or raise a ValueError? capacity = HPYUNICODEBUILDER_INITIAL_CAPACITY; } - capacity++; // always reserve space for an extra handle, see the docs, analogue to HPyTracker + capacity++; // always reserve space for the trailing 0 bp = malloc(sizeof(_PyUnicodeBuilder_s)); if (bp == NULL) { @@ -46,34 +48,37 @@ ctx_UnicodeBuilder_New(HPyContext *ctx, HPy_ssize_t capacity) return _pb2hb(0); } - bp->list = PyList_New(capacity); - if (bp->list == NULL) { + bp->buf = calloc(1, capacity); + if (bp == NULL) { free(bp); HPyErr_NoMemory(ctx); return _pb2hb(0); } + bp->capacity = capacity; bp->length = 0; return _pb2hb(bp); } _HPy_HIDDEN int -ctx_UnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, HPy h_item) +ctx_UnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, const char *item) { - if(!HPyUnicode_Check(ctx, h_item)) { - HPyErr_SetString(ctx, ctx->h_TypeError, "Argument must be of type HPyUnicode"); - return -1; - } - _PyUnicodeBuilder_s *bp = _hb2pb(builder); - PyObject *item = _h2py(h_item); - - // XXX: For the initial PoC we don't care about reallocation - if (bp->capacity <= bp->length) { - return -1; + // TODO: Should we trust the user to submit a 0 terminated string? + // The alternative would be to use strnlen and have a maximum allowed length for s + int len = strlen(item); + if(bp->length + len >= bp->capacity) { + // TODO: Have a better reallocation strategy + int new_size = bp->capacity + len + 1; + bp->buf = realloc(bp->buf, new_size); + if(bp->buf == NULL) { + free(bp); + HPyErr_NoMemory(ctx); + return -1; + } } - Py_INCREF(item); - PyList_SET_ITEM(bp->list, bp->length++, item); + strncpy((bp->buf + bp->length), item, (bp->capacity - bp->length)); + bp->length += len; return 0; } @@ -81,27 +86,16 @@ _HPy_HIDDEN HPy ctx_UnicodeBuilder_Build(HPyContext *ctx, HPyUnicodeBuilder builder) { _PyUnicodeBuilder_s *bp = _hb2pb(builder); - PyObject *list = PyList_GetSlice(bp->list, 0, bp->length); - - PyObject *sep = PyUnicode_FromString(""); - PyObject *str = PyUnicode_Join(sep, list); - Py_XDECREF(sep); - - if(str == NULL) { - PyErr_NoMemory(); - return HPy_NULL; - } - - Py_XDECREF(bp->list); - Py_XDECREF(list); + HPy h_result = HPyUnicode_FromString(ctx, bp->buf); + free(bp->buf); free(bp); - return _py2h(str); + return h_result; } _HPy_HIDDEN void ctx_UnicodeBuilder_Cancel(HPyContext *ctx, HPyUnicodeBuilder builder) { _PyUnicodeBuilder_s *bp = _hb2pb(builder); - Py_XDECREF(bp->list); + free(bp->buf); free(bp); } diff --git a/hpy/tools/autogen/public_api.h b/hpy/tools/autogen/public_api.h index f77c951b5..d68bf0994 100644 --- a/hpy/tools/autogen/public_api.h +++ b/hpy/tools/autogen/public_api.h @@ -284,7 +284,7 @@ HPy HPyListBuilder_Build(HPyContext *ctx, HPyListBuilder builder); void HPyListBuilder_Cancel(HPyContext *ctx, HPyListBuilder builder); HPyUnicodeBuilder HPyUnicodeBuilder_New(HPyContext *ctx, HPy_ssize_t size); -int HPyUnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, HPy h_item); +int HPyUnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, const char *item); HPy HPyUnicodeBuilder_Build(HPyContext *ctx, HPyUnicodeBuilder builder); void HPyUnicodeBuilder_Cancel(HPyContext *ctx, HPyUnicodeBuilder builder); diff --git a/test/test_hpyunicodebuilder.py b/test/test_hpyunicodebuilder.py index 6b30285d1..8bb1f9c4d 100644 --- a/test/test_hpyunicodebuilder.py +++ b/test/test_hpyunicodebuilder.py @@ -3,49 +3,21 @@ class TestString(HPyTest): def test_unicode_builder(self): mod = self.make_module(""" - HPyDef_METH(f, "f", f_impl, HPyFunc_O) - static HPy f_impl(HPyContext *ctx, HPy h_self, HPy h_arg) + HPyDef_METH(f, "f", f_impl, HPyFunc_NOARGS) + static HPy f_impl(HPyContext *ctx, HPy h_self) { HPyUnicodeBuilder builder = HPyUnicodeBuilder_New(ctx, 0); if(HPy_IsNull(builder)) { HPyErr_SetString(ctx, ctx->h_RuntimeError, "Could not create HPyUnicodeBuilder"); return HPy_NULL; } - HPy h_s1 = HPyUnicode_FromString(ctx, "hello "); - HPy h_s2 = HPyUnicode_FromString(ctx, "!"); - HPyUnicodeBuilder_Add(ctx, builder, h_s1); - HPyUnicodeBuilder_Add(ctx, builder, h_arg); - HPyUnicodeBuilder_Add(ctx, builder, h_s2); + HPyUnicodeBuilder_Add(ctx, builder, "hello "); + HPyUnicodeBuilder_Add(ctx, builder, "world"); + HPyUnicodeBuilder_Add(ctx, builder, "!"); HPy h_string = HPyUnicodeBuilder_Build(ctx, builder); - HPy_Close(ctx, h_s1); - HPy_Close(ctx, h_s2); return h_string; } @EXPORT(f) @INIT """) - assert mod.f("world") == "hello world!" - - def test_type_error(self): - mod = self.make_module(""" - HPyDef_METH(f, "f", f_impl, HPyFunc_O) - static HPy f_impl(HPyContext *ctx, HPy h_self, HPy h_arg) - { - HPyUnicodeBuilder builder = HPyUnicodeBuilder_New(ctx, 0); - if(HPy_IsNull(builder)) { - HPyErr_SetString(ctx, ctx->h_RuntimeError, "Could not create HPyUnicodeBuilder"); - return HPy_NULL; - } - HPy h_long = HPyLong_FromLong(ctx, 42); - HPyUnicodeBuilder_Add(ctx, builder, h_long); - HPy_Close(ctx, h_long); - HPyUnicodeBuilder_Cancel(ctx, builder); - return HPy_NULL; - } - @EXPORT(f) - @INIT - """) - import pytest - with pytest.raises(TypeError) as exc_info: - mod.f("world") - assert exc_info.match("Argument must be of type HPyUnicode") + assert mod.f() == "hello world!"