Skip to content

[CLIENT-681] Optimize performance when using bytes across the client API by reducing the number of heap allocations#779

Open
DomPeliniAerospike wants to merge 76 commits into
devfrom
CLIENT-681-2025
Open

[CLIENT-681] Optimize performance when using bytes across the client API by reducing the number of heap allocations#779
DomPeliniAerospike wants to merge 76 commits into
devfrom
CLIENT-681-2025

Conversation

@DomPeliniAerospike
Copy link
Copy Markdown
Contributor

@DomPeliniAerospike DomPeliniAerospike commented May 27, 2025

Replaces the static pool with a dynamically allocated byte pool.

TODO

  • Aim for at least 80% code coverage
  • Document where this optimization is applied, if not everywhere
  • There might be an issue where dynamic_pool.h code is duplicated and increases the wheel size. Need to double check

Additional tickets:
CLIENT-3501

DomPeliniAerospike and others added 5 commits May 23, 2025 06:09
* Replace static_pool with dynamic_pool

CLIENT-681
Replaced static_pool with dynamic_pool
Change as_bytes from heap to stack for operations and expressions.

* Added an expanding block size and pointer block

CLIENT-681
Added an expanding block size and pointer block to avoid several realloc calls

* CLIENT-681
Change Dynamic_pool_resize, dynamic_pool_init, and pool_destroy to inline functions rather than macros
Ran linter and fixed flake issues
Renamed dynamic_pool.h to dynamic_pool.hpp to solve precommit issues

* Change dynamic_pool.h to dynamic_pool.hpp

To fix clang-format issues

* Replace static_pool with dynamic_pool

CLIENT-681
Replaced static_pool with dynamic_pool
Change as_bytes from heap to stack for operations and expressions.

* Added an expanding block size and pointer block

CLIENT-681
Added an expanding block size and pointer block to avoid several realloc calls

* CLIENT-681
Change Dynamic_pool_resize, dynamic_pool_init, and pool_destroy to inline functions rather than macros
Ran linter and fixed flake issues
Renamed dynamic_pool.h to dynamic_pool.hpp to solve precommit issues

* Change dynamic_pool.h to dynamic_pool.hpp

To fix clang-format issues

* Update cdt_list_operate.c

* changed pool_destroy to true for querys and scans

changed pool_destroy to true for querys and scans

* Testing error conditions

* FF to Dev (#620)

* [CLIENT-1837] CI/CD: Label version with git commit when building on a non-tagged commit (#605)

* Also label version when building unoptimized, with macOS debug support, or with uncommitted changes to the repo (i.e add a "dirty" label)
* Rename config.conf to config.conf.template and add config.conf to .gitignore, so creating and modifying config.conf won't cause the repo to be in a dirty state

* Auto-bump version to 15.0.1rc3.dev1 [skip ci]

* [DEVOPS-234] CI/CD: Build manylinux wheels and upload them to JFrog on every push to an arbitrary branch (#606)

* CI/CD: Add option to build and upload unoptimized wheels to JFrog for QE

* Auto-bump version to 15.0.1rc3.dev2 [skip ci]

---------

Co-authored-by: juliannguyen4 <109386615+juliannguyen4@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Refactored CLIENT-681

Refactored dynamic_pool.hpp to increase readability.
Fixed memory bug with dynamic_pool in queries and scans.
Added allocate_buffer as a variable to necessary functions.

* Update test_query_apply.py

* Update dynamic_pool.hpp

* Update dynamic_pool.hpp

* Corrected dynamic_pool

* Update apply.c

* Update type.c

* Resetting state

* Use manylinux builds for valgrind tests

* oop

* Update serializer.c

* Fixed as_bytes_init_wrap to use heap_b when needed

* Fixed memory leak in serializer.c

removed allocate_buffer parameter from serialize_based_on_serializer_policy.
Added true for freeBytes in destroy_dynamic_pool in put call.

* Update serializer.c

* Update serializer.c

* fixed calloc typo

* Modified serializer to only get bytes from bytes pool if there is no error

---------

Co-authored-by: juliannguyen4 <109386615+juliannguyen4@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Change the descriptive units
Added documentation
Removed redundant code
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 27, 2025

Codecov Report

❌ Patch coverage is 95.41779% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.59%. Comparing base (ce62d2e) to head (0ab9029).

Files with missing lines Patch % Lines
src/main/convert_expressions.c 79.16% 5 Missing ⚠️
src/main/dynamic_pool.c 93.84% 4 Missing ⚠️
src/main/conversions.c 95.77% 3 Missing ⚠️
src/main/serializer.c 81.25% 3 Missing ⚠️
src/main/client/set_xdr_filter.c 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #779      +/-   ##
==========================================
+ Coverage   84.44%   84.59%   +0.15%     
==========================================
  Files          99      100       +1     
  Lines       14062    14233     +171     
==========================================
+ Hits        11874    12040     +166     
- Misses       2188     2193       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/main/client/operate.c
Comment thread test/new_tests/test_cdt_index.py Outdated
@juliannguyen4 juliannguyen4 changed the title CLIENT-681: Create dynamically allocate byte pool. [CLIENT-681] Optimize usage of bytes across the client API by reducing the number of heap allocations using a dynamically allocated byte pool Jun 1, 2026
@juliannguyen4 juliannguyen4 changed the title [CLIENT-681] Optimize usage of bytes across the client API by reducing the number of heap allocations using a dynamically allocated byte pool [CLIENT-681] Optimize performance when using bytes across the client API by reducing the number of heap allocations Jun 1, 2026
@juliannguyen4
Copy link
Copy Markdown
Collaborator

juliannguyen4 commented Jun 1, 2026

Valgrind reporting a memory error at this test case at current HEAD: new_tests/test_operate_helpers.py -k "test_pos_operate_prepend_with_serializer_fail[key1-llist1]"

Valgrind also reports memory error with this test case at a52315e

Memory error addressed at cf5e31c

…trictypes() is called, as_operations ops's number of operations is manually incremented, but the as_binop's bin field is undefined.
…_dynamic_pool_free_table to as_dynamic_pool_destroy. TODO - risk is as_dynamic_pool_destroy may not be inlined because the amount of code in it has increased.
…o it isn't copied over to other source files
… in aerospike.c, but not defined in that same file. Removing static allows as_dynamic_pool_get_as_bytes to be defined in another source file i.e dynamic_pool.c
… as well. Since we moved as_dynamic_pool_get_as_bytes's definition to a source file, don't make it inline anymore (here, we trust the compiler to optimize this function properly)
… var, we need to define as_dynamic_pool in the header file so the struct size is known
… Add missing decl for as_dynamic_pool_destroy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants