Add memory allocation API with templated C++ delegates & minor fixups #1123

2022-01-31T11:03:26-08:00

KitsuneAlex commented

2022-01-31 11:03:26 -08:00

(Migrated from github.com)

This simple pull request adds a memory allocation API to the Yoga C API,
as well as the respective templated C++ delegate functions to allow for the use
of custom memory allocator alongside Yoga.

This simple pull request adds a memory allocation API to the Yoga C API, as well as the respective templated C++ delegate functions to allow for the use of custom memory allocator alongside Yoga.

KitsuneAlex commented

2022-01-31 12:43:13 -08:00

(Migrated from github.com)

I also think it might be sensible to remove the YG name prefix from all functions within Yoga.h which are C++-only
and to move them into the yoga namespace.

I also think it might be sensible to remove the YG name prefix from all functions within Yoga.h which are C++-only and to move them into the **yoga** namespace.

KitsuneAlex commented

2022-02-02 15:02:30 -08:00

(Migrated from github.com)

Updated some things, test-compiled it on Windows x86_64 (under both Clang and MSVC), Linux x86_64 and MacOS x86_64 and ARM64 and everything's looking good.

KitsuneAlex commented

2022-02-02 15:20:19 -08:00

(Migrated from github.com)

Found another problem regarding static libraries and relocation on GCC/Clang, fixed that as well.

NickGerleman (Migrated from github.com) requested changes 2022-10-03 09:27:55 -07:00

NickGerleman (Migrated from github.com) left a comment

I think letting Yoga use a custom allocator is a great feature.

I left some comments on the PR wrt implementation.

I think letting Yoga use a custom allocator is a great feature. I left some comments on the PR wrt implementation.

yoga/Yoga.cpp

						
				@@ -0,0 +114,4 @@

				})

				#else

				#define YG_ALLOC_ALIGNED ([](size_t a, size_t s) -> void* { \

				  return aligned_alloc(a, s);                               \

NickGerleman (Migrated from github.com) commented

2022-10-03 09:20:49 -07:00

nit: why are these macros vs a regular function?

yoga/Yoga.cpp

						
				@@ -0,0 +130,4 @@

				  gAllocatorAllocateFunc = allocFunc;

				  gAllocatorFreeFunc = freeFunc;

				}

NickGerleman (Migrated from github.com) commented

2022-10-03 09:19:44 -07:00

If these are global, they will need to be protected by mutex, in case multiple threads are using Yoga at once.

I think it would be cleaner if the allocator was set as part of YGConfig, though it would mean YGConfig itself could not be allocated using the custom allocator. Though, Yoga may otherwise call functions which perform their dynamic allocations, so if the goal is just for the large structures, I think it could still be okay.

If these are global, they will need to be protected by mutex, in case multiple threads are using Yoga at once. I think it would be cleaner if the allocator was set as part of YGConfig, though it would mean YGConfig itself could not be allocated using the custom allocator. Though, Yoga may otherwise call functions which perform their dynamic allocations, so if the goal is just for the large structures, I think it could still be okay.

yoga/Yoga.cpp

						
				@@ -0,0 +132,4 @@

				}

				YOGA_EXPORT void YGGetAllocationCallbacks(

				    YGAllocatorAllocateFunc* allocFunc,

NickGerleman (Migrated from github.com) commented

2022-10-03 09:25:11 -07:00

YOGA_EXPORT void YGSetAllocator(

```suggestion YOGA_EXPORT void YGSetAllocator( ```

yoga/Yoga.cpp

						
				@@ -0,0 +150,4 @@

				  gAllocatorFreeFunc(memory);

				}

				static inline bool YGDoubleIsUndefined(const double value) {

NickGerleman (Migrated from github.com) commented

2022-10-03 09:23:45 -07:00

nit: Do we need to expose allocate and free as part of the public Yoga API? This already has an API for getting the allocators which are set. It doesn't seem like a good code pattern for folks to use Yoga's APIs for their own memory allocation.

yoga/Yoga.h

NickGerleman (Migrated from github.com) commented

2022-10-03 09:26:34 -07:00

This should not be part of the public API, since it is a helper for the internals of Yoga.

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#1123