Revert "eina: remove no longer used function _eina_thread_queue_msg_block_real_free"
This reverts commit 695b44526c.
Revert "eina/threadqueue: use mempool_del for hash free function"
This reverts commit b0cb3b935a.
Revert "eina_thread_queue: use normal mempools for block allocation"
This reverts commit 14ae3e3dec.
Why? Threadqueue is a highly performance sensitive API.
_eina_thread_queue_msg_block_new() may be called quite often. Doing a
hash lookup to then find a mempool handle to then allocate from was
not the same as what was there and was going to be far more costly.
This would have actual performance impact as we have to compute a hash
and rummage through a hash, hunt for an environment var too. The
original code looked at a spare block pool where blocks *MAY* be of
different sizes (not always the same size so using a mempool is
actually wrong and will stop threadqueue from being able to send
larger messages at all). If you send large messages, larger blocks would
have been allocated and put in this pool. In almost all cases the first
item in the pool would be big enough so we don't hunt and the find pulls
out the first memory, resets the fields that are needed and returns that
block. If it needs a bigger one, it does hunt. This is going to be
rare that such big blocks are needed so I never tried to optimize this
(but it could be done with an array of sizes to make a walk to find
the right sized element cheap if the need arises).
Performance dropped quite a lot. On aarch64 The above mempool usage
dropped message rate from 1037251 msg/sec to 610316. On x86 it was even
worse. It dropped from 2815775 msg/sec to 378653.
So backing this out sees the message rate is 7.4 times faster and on
aarch64 it's 1.7 times faster.
So moving to a mempool was actually just wrong (size is not always the
same). Also this ended up with a mempool of 64k for thread queue blocks even
if we only sent messages sporadically, as opposed to a single 4kb
block. So backing this out saves memory by only having 1 or 2 4k blocks
around most of the time, not a 64k mempool.
So the above patch then follow-on patches were done without accounting
for the performance implications. There were good reasons to do what I
did - because this code was highly tuned even to the point where I
used atomics instead of locks specifically to cut down some contention
overhead. Beware when you change something that there may be steep
performance implications. 7.4 times faster to go back to what was
there is a great example.
Summary:
It seems this function is no longer used since some rework. Clean it up.
lib/eina/eina_thread_queue.c:127:1: warning: ‘_eina_thread_queue_msg_block_real_free’ defined but not used [-Wunused-function]
Reviewers: devilhorns
Reviewed By: devilhorns
Subscribers: cedric, #committers, zmike
Tags: #efl
Differential Revision: https://phab.enlightenment.org/D6537
Summary:
I typod this in 14ae3e3dec and when using
mempools other than chained, this probably caused all apps to crash on
shutdown
Reviewers: ManMower, devilhorns
Reviewed By: ManMower
Subscribers: cedric, #committers
Tags: #efl
Differential Revision: https://phab.enlightenment.org/D6428
there's no need to reimplement mempools here when we already have a great
mempool api
this makes the code slightly more readable
Differential Revision: https://phab.enlightenment.org/D6353
Summary:
the 'first' member of this struct is used simultaneously across
threads and can have conflicting read/write operations occurring at this
time
as int operations are not guaranteed to be atomic, ensure that we are
using atomic operations or locking as necessary
@fix
Depends on D6299
Reviewers: ManMower, devilhorns
Reviewed By: ManMower
Subscribers: cedric, #committers
Tags: #efl
Differential Revision: https://phab.enlightenment.org/D6300
Summary:
this moves lock calls outside #ifdef blocks to make the code more
readable
no functional changes
Depends on D6298
Reviewers: ManMower, devilhorns
Reviewed By: ManMower
Subscribers: cedric, #committers
Tags: #efl
Differential Revision: https://phab.enlightenment.org/D6299
Summary:
this would deadlock in some cases
@fix
Depends on D6297
Reviewers: ManMower, devilhorns
Reviewed By: ManMower
Subscribers: cedric, #committers
Tags: #efl
Differential Revision: https://phab.enlightenment.org/D6298
This reverts commit d19cd4e63c.
This causes a SIGBUS error on OpenBSD when closing any application. As this
was safety patch only I will revert it for the 1.18 release and we can work
out what breaks OpenBSD here for 1.19
Fixes T4332
so the spinlock on the threadqueue block pool it taken on shutdownn,
while the block pool is freed up then its is destroyed, but openbsd
very much doesnt like this and returns an error, so release the lock
before destroying it.
@fix
coverity pointed out a deadlock - CID 1226193. i hope this fixes it
(need to wait days for a coverity re-run). tests pass and real life
usage works, so i assume its ok. in theory i see no problem either.
@feature
This is a new feature for eina (and EFL) - a zero-copy thread message
queue for sending messages from one thread to another or from the
ecore mainloop to or back to the mainloop from threads. It has a
complete test suite too.