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.

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

Overhaul allocator framework #133

Closed
wants to merge 257 commits into from
Closed

Conversation

inakleinbottle
Copy link
Contributor

Built up the allocator framework within RoughPy_Platforms to centralise all the allocators and container types that are used throughout. Closes #129.

This change moves the template declaration above the documentation comment for the make_rc function. This ensures that the template parameters for T and Args are correctly recognized and associated with the function.
Added ignore_unused macro to old_rc variable in the RcBase::inc_ref() method. This change prevents compiler warnings about the unused variable while maintaining the existing logic.
Move "errors.h" include to correct position for better organization. This change ensures consistency in the header file and potentially reduces compilation errors.
This change improves compilation efficiency by ensuring the file is only included once. The existing macro guards remain as a fallback for older compilers.
Introduced the is_pointer_aligned function to check pointer alignment. Also reorganized code by moving alignment-related functions into a new namespace, align, for better structure and readability. Updated AlignedAllocator to use the new namespace.
Ensured that the alignment is validated by adding a debug assertion before memory allocation. This helps to catch potential misalignment issues early during development and debugging processes.
This commit adds unit tests for the aligned allocator in the platform's memory module. New tests ensure proper memory alignment and handling of zero-size allocations using Google Test. The test configuration is included in CMakeLists.txt, and a new test file test_aligned_allocator.cpp is added.
Included new test file `test_small_object_allocator.cpp` in the memory module's CMake configuration. This test ensures that the upstream allocator of the SmallObjectAllocator is set correctly.
Created unit tests to verify reference counting mechanisms, including incrementing and decrementing references, custom implementations, and behavior upon resetting and releasing references. Updated CMakeLists.txt to include the new test file.
Shifted the template declaration for `make_rc` to follow the Doxygen comment. This improves code readability and maintains consistency with the documentation style in the codebase.
Deleted two superfluous blank lines from the align namespace section to improve readability and maintain code consistency. This change does not affect functionality but aligns with style guidelines.
This commit removes the RPY_UNUSED attribute from the program() and context() methods in the ocl_kernel.h file. These methods are now marked as regular member functions, indicating they are actively used in the codebase.
Updated RcBase methods to be noexcept and clarified std::nullptr_t usage in Rc constructor. Additionally, fixed a friend class declaration typo for better clarity and maintainability.
This commit marks the small_object_alloc function as noexcept to indicate it does not throw exceptions, enhancing code reliability and optimization. It also removes the noexcept specifier from RcBase::dec_ref to allow exception propagation, ensuring accurate ref-count management and potential debugging aids.
Update the alignment check to use the `align::is_alignment` function. This ensures consistency and avoids potential namespace issues.
Changed `#else if` to `#elif` for GCC and Clang preprocessor directives to ensure proper conditional compilation. This fixes issues related to aligned memory allocation and deallocation on platforms using GCC or Clang compilers.
The RPY_UNUSED macro in the compare_same function declaration was unnecessary and has been removed. This change enhances code readability and ensures the removal of redundant code elements.
The RPY_UNUSED macro was removed from the RPyContext_new function declaration as it was unnecessary. This change helps in simplifying the code and improving readability.
Removed the `RPY_UNUSED` annotations from `size`
The 'RPY_UNUSED' attribute was removed from the static function 'lie_increment_path_from_values' in lie_increment_stream.cpp. This cleanup does not affect functionality but improves code readability.
The `RPY_UNUSED` attribute was removed from the `dlpack_dtype_to_scalar_type` function's declaration. This cleanup helps to reduce unnecessary attributes from the codebase, improving readability and maintainability.
Updated memory allocation functions to use new namespaces: 'align' and 'small' instead of 'dtl'. Additionally, included the appropriate headers for 'aligned_alloc' and 'small_object_alloc' functionalities to ensure correct linkage and utility.
Reordered parameters in `small_object_alloc` for consistency with other functions. Added `small_object_free` to manage deallocation of small objects, enhancing memory management.
Switched to using namespace-scoped `align::aligned_alloc` and `align::aligned_free` for memory allocation and deallocation. This change improves code readability and ensures the correct function is used within the align namespace.
Refactor RcRefCountingTest by utilizing aliases to streamline code clarity and fixing the object initialization within each test. Improve aligned_alloc with enhanced error handling and dependency cleanup. Remove unnecessary debug assertions in inc_ref() and update test_small_object_allocator for consistency.
Changed the second parameter of the raw_alloc call from `sizeof(float)` to `alignof(float)` for more accurate memory alignment. This ensures the allocated memory aligns with the requirements of the float type.
Replace `bit_cast` with `reinterpret_cast` for pointer alignment check to fix potential compatibility issues. Also, removed extraneous blank lines in test_ref_count_pointer.cpp for improved code readability.
Centralize memory-related functionality under the mem namespace instead of rpy. This change affects functions, classes, and tests to enhance code organization and readability.
Updated `aligned_alloc` and `aligned_free` functions to be accessed from the `mem::align` namespace. This change improves code clarity and ensures consistency with the rest of the memory management utilities.
Update function signatures to use namespace 'mem' for consistency and rename return types to 'mem::small::PoolMemory'. This change ensures that the memory resource functions align with the correct namespace and type definitions, improving code clarity and organization.
This commit removes a conditional block that copied runtime DLLs for Windows targets in the `roughpy_helpers.cmake` file. The functionality was redundant and has been cleaned up to streamline the post-build process.
Set the `CMAKE_GTEST_DISCOVER_TESTS_DISCOVERY_MODE` to `PRE_TEST` for GTest. This ensures tests are discovered before running them, improving build consistency.
Added custom post-build commands for Windows in CMakeLists.txt to copy runtime DLLs for test targets. Removed obsolete functions from roughpy_helpers.cmake and re-enabled post-build commands for test targets.
Changed the target name in the POST_BUILD custom command from a variable to 'RoughPy_Platform_test_memory'. This ensures that the correct executable is referenced, which resolves an issue with copying runtime DLLs on Windows.
Added comments to custom commands in CMakeLists to clarify DLL copying actions for `RoughPy_Platform_test_memory`. This change improves code readability and simplifies future debugging efforts.
Modified `CMakeLists.txt` and `roughpy_helpers.cmake` to include environment path adjustments for test executables. This ensures that the test processes can locate the required DLLs properly by prepending the target directory to the PATH.
Adjusted the CMAKE_PREFIX_PATH to include parent directory of Pybind11 installation for correct directory resolution. Changed the find_package command to use the CONFIG mode, ensuring proper configuration file lookup.
Correct the `if` conditions to properly reference `_vcpkg_dir` and handle the definition of `pybind11_ROOT`. This ensures accurate detection and necessary actions when certain directories and variables are absent or undefined.
Comment out the custom triplet line in CMakeLists.txt and add the overlay triplets configuration in vcpkg-configuration.json. This change ensures the triplets directory is properly recognized by vcpkg.
Deleted the `add_roughpy_algebra` function due to redundancy and commented out a test environment modification loop that caused inconsistent behavior. These changes aim to simplify the build configuration and prevent potential test execution issues.
Commented out the foreach loop that was setting PATH environment modification for RoughPy_Platform_test_memory_TESTS. This change aims to address issues related to environment path conflicts during testing.
Corrected the alignment parameter in the _aligned_malloc function call for MSVC builds from 'size' to 'alignment'. This ensures that the memory allocations adhere to the specified alignment requirement properly, preventing potential memory misalignment issues.
The boost-stacktrace dependency has been removed from vcpkg.json. This change reduces the number of included libraries, which might help simplify the build process and reduce potential conflicts. Other dependencies remain unchanged, ensuring that all required features are still supported.
This change ensures that CMake always prioritizes finding a virtual environment before other Python installations. Additionally, it removes redundant conditional statements for setting the virtual environment and adjusts a condition
Introduce a new script `dependency_finder.py` that scans a JSON file to identify packages depending on a specified dependency. This aids in dependency management by allowing users to trace direct dependents of a given package.
The script to find dependencies on boost-fusion is removed. It was generated by AI for a specific task and is no longer needed in the project. Additionally, its functionality can be achieved through more robust methods.
Changed the `baseline` in `vcpkg-configuration.json` from `b9fe496e225dfe6ee362f2382279654e3d6eb618` to `4f746bc66438fce2b900c3ba6094a483b871b045`. This ensures the project pulls the latest changes from the vcpkg repository.
Extended the `PoolAllocator` and `AlignedAllocator` classes by including detailed type definitions and additional methods, such as `propagate_on_container_*` traits and `is_always_equal`. Also moved memory allocation and deallocation logic for `AlignedAllocator` into method definitions to improve readability and maintainability.
Replaced temporary vector allocation with aligned allocator in hall_set_size.cpp for improved performance. Modified allocator traits in memory.h to disable propagation on container operations, ensuring custom allocator consistency.
Modified the `AlignedAllocator` template to use `alignof(std::max_align_t)` instead of `alignof(Ty)`. This change ensures that memory allocations are correctly aligned according to the largest possible alignment requirements on the target platform.
Replaced custom memory allocators with std::allocator in various container templates across the codebase. This change aims to standardize memory allocation practices and improve code maintainability.
Changed the static instance in `AlignedMemory::get()` to use `std::make_unique`, ensuring better management of the singleton's lifetime and potential thread-safety improvements. This modification eases the handling of dynamic memory allocation within the function.
Replaced raw pointer with smart pointer for small object memory resource. This ensures better memory management and prevents potential memory leaks.
Ensure all memory allocation and deallocation functions use the fully qualified names across different compilers. This change guards against potential name clashes and ensures clearer linkage to standard library functions.
This commit introduces a static constexpr member `mem_alignment` for consistent memory alignment in the `StandardScalarType` class. It updates the memory allocation in the `allocate` method to use this fixed alignment.
Refine the label comparison loop by adding a termination condition using the actual end of the `item_label`, enhancing clarity and safety. Includes a TODO comment suggesting further improvement with constrained algorithms.
Replaced iterators with pointers obtained from data() and size() methods to simplify and potentially improve the efficiency of the from_chars function call. This change is expected to enhance code readability and maintainability.
@inakleinbottle
Copy link
Contributor Author

OK it's quite clear that Windows is going to continue causing many, many problems. This is too many changes to debug successfully, so I'm shelving this for now and I'll work on this in smaller parts that can be tested much more easily.

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.

RoughPy Allocators and standard containers overhaul
1 participant