Closed mjklemm closed 2 years ago
@JimCownie This is not yet ready to go, but you may want to have a lock at this.
Rather than putting the second definition in the
#else
, since other target deoendent things may end up in there, and this makes the separation clearer IMO
Good point. I will have to see though how this relates to the other possible scheduling implementations. That part of the tasking runtime needs some refactoring regardless of this. I did not yet get to making these things configurable via the CMakery.
This all seems overly complicated to me. Can't you overload the new and delete operators in the classes that want aligned allocation so that they use an aligned allocator? Then you don't then need to change every place where the class object is created or destroyed. That's how it was being done in barrier_impl.cc with
class alignedAllocators
I wanted to keep the allocation logic separated from the object itself. I guess we could have local overloads of new/delete within the class that then forward to the proper allocation logic in memory.h.
I much prefer a scheme in which the alignment of the object is an internal property, rather than something which affects every new/delete site. That says nothing about how the aligned allocation is actually done, which can be certainly be shared, but (as your patch currently shows) if it's not the case you have to change lots of places when you add alignment to an object, whereas if you can keep the simple new/delete view for the user of the class adding alignment is much easier. And, philosophically, the alignment property seems something that matters to the class itself but shouldn’t be an issue for its users.
On 23 Feb 2022, at 10:46, Michael Klemm @.***> wrote:
This all seems overly complicated to me. Can't you overload the new and delete operators in the classes that want aligned allocation so that they use an aligned allocator? Then you don't then need to change every place where the class object is created or destroyed. That's how it was being done in barrier_impl.cc with class alignedAllocators
I wanted to keep the allocation logic separated from the object itself. I guess we could have local overloads of new/delete within the class that then forward to the proper allocation logic in memory.h.
— Reply to this email directly, view it on GitHub https://github.com/parallel-runtimes/lomp/pull/55#issuecomment-1048654353, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALY7TVEALJXZWGEVY6TSP73U4S3IFANCNFSM5PCABPJQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you were mentioned.
-- Jim James Cownie @.***> Mob: +44 780 637 7146
I much prefer a scheme in which the alignment of the object is an internal property, rather than something which affects every new/delete site.
Ok, that works for me. I made such an update. I kept the make_* functions in memory.h. They can now be used as the back-end allocation functions or for cases, where we'd like to keep things out of the class (or where this cannot be done easily, like the TaskPool class).
Introduce memory allocation functions
make_aligned
,make_aligned_struct
, andmake_aligned_chunk
(plus their deallocation counterparts) to perform memory allocations that are aligned toCACHELINE_SIZE
(seetarget.h
) bytes.Resolves issue #7.