[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #27208 [Core Tor/Tor]: add API for allocating aligned memory



#27208: add API for allocating aligned memory
-----------------------------+------------------------------------
 Reporter:  cyberpunks       |          Owner:  (none)
     Type:  enhancement      |         Status:  needs_revision
 Priority:  Medium           |      Milestone:  Tor: 0.3.6.x-final
Component:  Core Tor/Tor     |        Version:
 Severity:  Normal           |     Resolution:
 Keywords:  rust-wants rust  |  Actual Points:
Parent ID:  #23882           |         Points:
 Reviewer:                   |        Sponsor:
-----------------------------+------------------------------------
Changes (by dgoulet):

 * status:  needs_review => needs_revision


Comment:

 Thanks for this!

 * This code uses `posix_memalign()` as the fallback if both `memalign()`
 and `aligned_alloc()` aren't present. This has been added a _while_ back
 so I'm not too worried about any system we currently build on today to not
 have it but hey you never know... (`_POSIX_C_SOURCE >= 200112L`).

  Should we add a check at configure time and fail if none of the 3
 functions are present?

 * I would add a `PREDICT_UNLIKELY()` around the `posix_memalign()` for
 sake of "performance" :).

 * I think we should make `static void *aligned_alloc...` inline in my
 opinion... might need some code change from the patch but since
 `tor_malloc()` is used extensively, any performance gain is good.

 * More a meta-physic question: Do we really need to reroute every
 `tor_malloc()` to the aligned one with alignment set to 1? In the case of
 `memalign()` and `aligned_alloc()`, of what I can read from the glibc, it
 will be re-routed to the normal `malloc()` if alignment is below some
 minimum.

  But with `posix_memalign()`, this means that every `tor_malloc()` now
 needs to do a `MAX()`, which results in _always_ using `sizeof(void *)`. I
 know this is very cheap CPU side but maybe we can avoid this couple of
 extra cycles by simply making `tor_malloc_()` use the right alignment by
 default and removing the `MAX()` ?

  I know I'm arguing about nanoseconds here but `tor_malloc()` is one of
 the *core* foundation of the entire code base, adding any extra work to it
 is something I personally want to avoid unless it is security related.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/27208#comment:4>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs