diff --git a/CHANGELOG.md b/CHANGELOG.md index d69a5e5d1..9b35631db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added support for external pids and encoded pids in external terms - Added support for external refs and encoded refs in external terms - Introduce ports to represent native processes and added support for external ports and encoded ports in external terms +- Added `externalterm_to_term_with_roots` to efficiently preserve roots when allocating memory for external terms. + +### Changed + +- Removed `externalterm_to_term_copy` added in [0.6.5] and introduced flags to `externalterm_to_term` to perform copy. ## [0.6.6] - Unreleased diff --git a/src/libAtomVM/externalterm.c b/src/libAtomVM/externalterm.c index 10a3028f7..b72b8fe60 100644 --- a/src/libAtomVM/externalterm.c +++ b/src/libAtomVM/externalterm.c @@ -30,6 +30,7 @@ #include "bitstring.h" #include "defaultatoms.h" +#include "memory.h" #include "term.h" #include "unicode.h" #include "utils.h" @@ -81,20 +82,8 @@ static size_t compute_external_size(term t, GlobalContext *glb); static int externalterm_from_term(uint8_t **buf, size_t *len, term t, GlobalContext *glb); static int serialize_term(uint8_t *buf, term t, GlobalContext *glb); -/** - * @brief - * @param external_term buffer containing external term - * @param size size of the external_term - * @param ctx current context in which terms may be stored - * @param opts additional opts, such as ExternalTermToHeapFragment for storing parsed - * terms in a heap fragment. - * are stored in the context heap. - * @param bytes_read the number of bytes read off external_term in order to yield a term - * @param copy whether to copy binary data and atom strings (pass `true', unless `external_term' is a const binary and will not be deallocated) - * @return the parsed term - */ -static term externalterm_to_term_internal(const void *external_term, size_t size, Context *ctx, - ExternalTermOpts opts, size_t *bytes_read, bool copy) +term externalterm_to_term_with_roots(const void *external_term, size_t size, Context *ctx, + ExternalTermFlags flags, size_t *bytes_read, size_t num_roots, term *roots) { const uint8_t *external_term_buf = (const uint8_t *) external_term; @@ -107,42 +96,36 @@ static term externalterm_to_term_internal(const void *external_term, size_t size } size_t eterm_size; - int heap_usage = calculate_heap_usage(external_term_buf + 1, size - 1, &eterm_size, copy); + int heap_usage = calculate_heap_usage(external_term_buf + 1, size - 1, &eterm_size, flags & ExternalTermCopy); if (heap_usage == INVALID_TERM_SIZE) { return term_invalid_term(); } term result; - if (opts & ExternalTermToHeapFragment) { + if (flags & ExternalTermToHeapFragment) { // We need to allocate fragments as reading external terms from modules // is not accounted for by the compiler when it emits test_heap opcodes Heap heap; if (UNLIKELY(memory_init_heap(&heap, heap_usage) != MEMORY_GC_OK)) { return term_invalid_term(); } - result = parse_external_terms(external_term_buf + 1, &eterm_size, copy, &heap, ctx->global); + result = parse_external_terms(external_term_buf + 1, &eterm_size, flags & ExternalTermCopy, &heap, ctx->global); memory_heap_append_heap(&ctx->heap, &heap); } else { - if (UNLIKELY(memory_ensure_free(ctx, heap_usage) != MEMORY_GC_OK)) { + if (UNLIKELY(memory_ensure_free_with_roots(ctx, heap_usage, num_roots, roots, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { fprintf(stderr, "Unable to ensure %zu free words in heap\n", eterm_size); return term_invalid_term(); } - result = parse_external_terms(external_term_buf + 1, &eterm_size, copy, &ctx->heap, ctx->global); + result = parse_external_terms(external_term_buf + 1, &eterm_size, flags & ExternalTermCopy, &ctx->heap, ctx->global); } *bytes_read = eterm_size + 1; return result; } -term externalterm_to_term(const void *external_term, size_t size, Context *ctx, ExternalTermOpts opts) -{ - size_t bytes_read = 0; - return externalterm_to_term_internal(external_term, size, ctx, opts, &bytes_read, false); -} - -term externalterm_to_term_copy(const void *external_term, size_t size, Context *ctx, ExternalTermOpts opts) +term externalterm_to_term(const void *external_term, size_t size, Context *ctx, ExternalTermFlags flags) { size_t bytes_read = 0; - return externalterm_to_term_internal(external_term, size, ctx, opts, &bytes_read, true); + return externalterm_to_term_with_roots(external_term, size, ctx, flags, &bytes_read, 0, NULL); } enum ExternalTermResult externalterm_from_binary(Context *ctx, term *dst, term binary, size_t *bytes_read) @@ -150,22 +133,24 @@ enum ExternalTermResult externalterm_from_binary(Context *ctx, term *dst, term b if (!term_is_binary(binary)) { return EXTERNAL_TERM_BAD_ARG; } - // - // Copy the binary data to a buffer (in case of GC) - // size_t len = term_binary_size(binary); const uint8_t *data = (const uint8_t *) term_binary_data(binary); - uint8_t *buf = malloc(len); - if (IS_NULL_PTR(buf)) { - fprintf(stderr, "Unable to allocate %zu bytes for binary buffer.\n", len); - return EXTERNAL_TERM_MALLOC; + term t; + if (term_is_heap_binary(binary)) { + // + // Copy the binary data to a buffer (in case of GC) + // + uint8_t *buf = malloc(len); + if (IS_NULL_PTR(buf)) { + fprintf(stderr, "Unable to allocate %zu bytes for binary buffer.\n", len); + return EXTERNAL_TERM_MALLOC; + } + memcpy(buf, data, len); + t = externalterm_to_term_with_roots(buf, len, ctx, ExternalTermCopy, bytes_read, 0, NULL); + free(buf); + } else { + t = externalterm_to_term_with_roots(data, len, ctx, ExternalTermCopy, bytes_read, 1, &binary); } - memcpy(buf, data, len); - // - // convert - // - term t = externalterm_to_term_internal(buf, len, ctx, false, bytes_read, true); - free(buf); if (term_is_invalid_term(t)) { return EXTERNAL_TERM_BAD_ARG; } else { diff --git a/src/libAtomVM/externalterm.h b/src/libAtomVM/externalterm.h index a1c7082d1..6cc931152 100644 --- a/src/libAtomVM/externalterm.h +++ b/src/libAtomVM/externalterm.h @@ -47,8 +47,9 @@ enum ExternalTermResult typedef enum { ExternalTermNoOpts = 0, - ExternalTermToHeapFragment = 1 -} ExternalTermOpts; + ExternalTermToHeapFragment = 1, + ExternalTermCopy = 2, +} ExternalTermFlags; /** * @brief Gets a term from external term data. @@ -57,30 +58,34 @@ typedef enum * @param external_term the external term that will be deserialized. * @param size to allocate for term. * @param ctx the context that owns the memory that will be allocated. - * @param opts if non-zero, use a heap fragment to store the generated - * terms. Otherwise, use the heap in the provided context. Note that when using the - * context heap, this function may call the GC, if there is insufficient space to - * store the generated terms. + * @param flags options to deserialize data. + * The following flags are supported: + * - `ExternalTermToHeapFragment' : use a heap fragment to store the generated + * terms (default is to use the heap in the provided context). Note that when + * using the context heap, this function may call the GC, if there is + * insufficient space to store the generated terms. + * - `ExternalTermCopy' : copy (binary and atom) data. Suitable when + * `external_term' pointer is not a const literal. * @returns a term. */ term externalterm_to_term( - const void *external_term, size_t size, Context *ctx, ExternalTermOpts opts); + const void *external_term, size_t size, Context *ctx, ExternalTermFlags flags); /** - * @brief Gets a term from external term data, and makes a copy of all data. - * - * @details Deserialize an external term from external format and returns a term. - * @param external_term the external term that will be deserialized. - * @param size to allocate for term. - * @param ctx the context that owns the memory that will be allocated. - * @param opts if non-zero, use a heap fragment to store the generated - * terms. Otherwise, use the heap in the provided context. Note that when using the - * context heap, this function may call the GC, if there is insufficient space to - * store the generated terms. - * @returns a term. + * @brief Create a term from external term data. This variant is meant to be used + * for distribution or in cases where the number of bytes read is important. + * + * @param external_term buffer containing external term + * @param size size of the external_term + * @param ctx current context in which terms may be stored + * @param flags additional flags (see above) + * @param bytes_read the number of bytes read off external_term in order to yield a term + * @param num_roots number of roots when invoking GC + * @param roots roots to preserve when invoking GC + * @return the parsed term */ -term externalterm_to_term_copy( - const void *external_term, size_t size, Context *ctx, ExternalTermOpts opts); +term externalterm_to_term_with_roots(const void *external_term, size_t size, Context *ctx, + ExternalTermFlags flags, size_t *bytes_read, size_t num_roots, term *roots); /** * @brief Create a term from a binary. diff --git a/src/libAtomVM/term.h b/src/libAtomVM/term.h index a1393444c..d5fc4ec8a 100644 --- a/src/libAtomVM/term.h +++ b/src/libAtomVM/term.h @@ -431,9 +431,9 @@ static inline bool term_is_binary(term t) } /** - * @brief Checks if a term is a binary + * @brief Checks if a term is a refc binary * - * @details Returns \c true if a term is a binary stored on the heap, otherwise \c false. + * @details Returns \c true if a term is a ref-counted binary, otherwise \c false. * @param t the term that will be checked. * @return \c true if check succeeds, \c false otherwise. */ @@ -449,6 +449,25 @@ static inline bool term_is_refc_binary(term t) return false; } +/** + * @brief Checks if a term is a heap binary + * + * @details Returns \c true if a term is a binary stored on the heap, otherwise \c false. + * @param t the term that will be checked. + * @return \c true if check succeeds, \c false otherwise. + */ +static inline bool term_is_heap_binary(term t) +{ + /* boxed: 10 */ + if (term_is_boxed(t)) { + const term *boxed_value = term_to_const_term_ptr(t); + int masked_value = boxed_value[0] & TERM_BOXED_TAG_MASK; + return masked_value == TERM_BOXED_HEAP_BINARY; + } + + return false; +} + static inline bool term_refc_binary_is_const(term t) { const term *boxed_value = term_to_const_term_ptr(t);