Commit Graph

7 Commits

Author SHA1 Message Date
Mathias Agopian
591c9f2454 Fix a crasher when allocating the last Job
Instead of returning nullptr, the pool allocator returned an
invalid pointer.
2018-11-06 13:44:35 -08:00
Mathias Agopian
e2896f4632 Set cache size to 64 bytes on all platforms
We used to assume 32-bytes cache lines when running on ARM 32-bit mode,
however, that was wrong because 32-bit mode doesn't change the cpu's
cache line. On all modern platforms we support, the cache line
is 64 bytes.

Set Job storage to 48 bytes on all platforms.
2018-11-06 13:21:09 -08:00
Mathias Agopian
68c6afa72e Fix reuse after free and API inconsistency in job system (#446)
* Fix a reuse after free in the job system

Jobs were destroyed and recycled while still in use
by wait() or run(). To fix this we introduce reference-counting of
jobs.

Jobs start with a ref-count of 1, which is decremented when a job
naturally finishes. Additionally, all user-facing methods acquire
a reference for the duration of the call.

* Fix an API inconsistency with JobSystem

JobSystem's API lets the user create jobs but not destroy them.
Jobs are destroyed automatically, without a way for the caller to
know when that happens.

We now explicitly enforce that jobs are no longer valid when
wait() returns. Multiple concurrent wait() are allowed however.

This is enforced by clearing the job pointer upon returning
from JobSystem::wait(Job* job).

* Rename linked-list put/get to push/pop

* Better fix for Job use after free

There was still a race condition where a run()'ed
job could be destroyed before wait() was called,
wait would then use a destroyed object.

The available APIs now are:

run() - runs and destroys a job
runAndWait() - run, then waits for and destroys a job
runAndRetain() - runs and keep a reference to the job
wait() - waits and destroys a job

wait() can only be used with a job obtained with runAndRetain().

* Get rid of unused code

This version of parallel_for has use-after-free issues anyways,
since we changed the semantics of run/wait/etc...

* Fix decRef() memory order

decRef() must ensure that all access to the 
object have happened before destroying it.

* Fix memory order in atomic linked list's pop()

It needs acquire semantic, since we want to make
sure that no read/write are reordered before the
pop() -- which returns an object to the caller.

* Fix memory order on runningJobCount

we needed acquire semantic when about to destroy
the last job -- it's similar to decRef.

* Comment usages of std::memory_order_*

* Fix AtomicFreeList A-B-A bug

Turns out AtomicFreeList was not immune to the ABA bug. W're fixing
it here by using a 64-bits CAS, which is available on aarch64 and armv7.
2018-11-05 19:12:13 -08:00
Mathias Agopian
d9ba3998d2 JobSystem now automatically free Jobs (#91)
* JobSystem now automatically free Jobs

Until now Job allocation used a linear allocator
strategy which required to “reset” the JobSystem
periodically — typically once per frame in
filament.

This is no longer required. We use a pool allocator
now, which doesn’t add much overhead. It does
use a spin-lock for thread-safety though, since
we assume very little contention, this shouldn’t
be a problem.

* Thread Safe Object Pool Allocator

A lock-less, thread-safe object pool allocator,
now used for storing JobSystem’s jobs allocations.
This gets rid of the spin-lock introduced in the
previous cl.
2018-08-13 22:25:15 -07:00
Mathias Agopian
a4851ed835 fix a bunch of clang-tidy warnings
fixed a couple actual real bugs (missing returns 
in operator=, wrong implicit bool conversion).

mostly added a bunch of explicit ctor.
2018-08-10 20:53:58 -07:00
Mathias Agopian
94c3623c1c clean-up formatting
Change-Id: I2071e53cceb93cbe02d6bdfa238aa6ce770b0534
2018-08-06 18:14:11 -07:00
Romain Guy
b3d758f3b3 Initial commit 2018-08-03 10:38:22 -07:00