* Remove redundant qualifiers in filament public headers
* remove redundant qualifiers in filament implementation
* remove redundant qualifiers in libutils public headers
* remove redundant qualifier for libutils implementation
* remove redundant qualifiers for libmath
* use is_same_v<> instead of is_same<>
* bring back Builder::name()
we keep Builder::name() on all object, and forward to the MixIn class
that does the implementation, so that we have correct documentation, and
better IDE completion.
* add missing const parameters in filament's implementation
* various source cleanup
- missing includes
- missing const
- C cast style
- superfluous inline keyword
The previous fix attempt didn't work on some test. There are no known
bugs with AtomicFreeList other than tripping TSAN, and it's unclear
that TSAN isn't at fault.
However, switching to using a mutex works fine and doesn't appear to
be slower (it's actually faster with synthetic benchmarks on macOS)
BUGS=[377369108]
* improve parallel_for a bit
We get about 40% performance increase. The gain comes from not having
to copy the JobData structure each time we create a job, by using a
new emplaceJob() method, we can create the structure directly into
its destination.
* avoid calling wakeAll() when possible
wakeAll() is very expensive and not always needed when a job finishes
because there may not be anyone waiting on that job.
We now maintain a waiter count per job, and use that to determine if
we need to notify or not.
And now that the JobSystem overhead is lower, we can decrease the size
of the jobs, which improves the load balancing.
* mActiveJobs fixes
some comments claimed mActiveJobs needed to be modified before or after
accessing the WorkQueue; this couldn't be correct because there were no
guaranteed global ordering with the workQueue.
- reduce the number of calls to notify_one() and notify_all().
notify_one() is not only called when running a new job, and
notify_all() only when a job finishes.
- don't hold the condition lock while calling notify_*(), as it is not
strictly needed, and because notify_*() can be very slow, there can
be a lot of contention on this lock as a result; blocking the whole
jobsystem thread pool.
- add a new version of run() that takes an opaque thread id that can
be retrieved from a job's execute function; this is especially
intended to be used by parallel_for(); it's just a more efficient
version of run() that avoids a hashmap lookup.
Overall these change yield a significant performance boost:
- running + waiting a job: +200%
- running many jobs: +150%
- running many jobs in parallel: +50%
This has caused issues and over time we have reduced the use of
spinlocks, it was only used in few places and we still have evidence
that it's causing ANRs.
We use utils::Mutex instead which is a low overhead mutex implementation
on Linux systems.
FIXES=[321101014]
Filament does not yet fully support threads with WASM, but this is a
baby step in that direction.
To enable experimental pthreads support, enable the WEBGL_PTHREADS CMake
option. This will enable pthreads support in `gltfio` and `utils`, which
is known to work, but not when served with GitHub Pages.
The web server must emit COOP, COEP and CORP headers, so our build
instructions now recommend the use of `emrun` for local testing.
This also changes our demos so that they do not use unpkg, which
does not work when using `emrun`, due to cross-origin restrictions.
We were decrementing activeJobCount after removing the job from the
queue, which could cause other threads in the pool to preempt us before
the decrement, causing them to spin forever trying to get a non-existant
job, until the decrement actually happened.
Now we always decrement first and fix-up the count if we couldn't get
a job from the queues. The race is inverted, and doesn't cause threads
to spin a long time.
fixes b/201100123
We were decrementing activeJobCount after removing the job from the
queue, which could cause other threads in the pool to preempt us before
the decrement, causing them to spin forever trying to get a non-existant
job, until the decrement actually happened.
Now we always decrement first and fix-up the count if we couldn't get
a job from the queues. The race is inverted, and doesn't cause threads
to spin a long time.
We reuse the current job for the right side instead of spawning a new
job, which has overhead. This also cuts in half the number of times we
need to copy the user functor.
A side effect of this flag was that we had to always use notify_all
when starting a job, instead of notify_one -- in practice, we found
experimentally that more calls to notify_one is cheaper than fewer
calls to notify_all.
The hang was caused by a subtle race. When a job is completed, its
thread must signal all the threads that might be waiting on this job.
The signaling code was attempting to signal only the minimum number
of threads -- this was important especially in the case where no threads
were waiting, then the call to notify() could be avoided.
Unfortunately, for performance reasons we're not calling notify() with
the condition lock held, this meant that between the time the number of
waiting threads was latched and the time of the notify() call, more
threads could enter their condition variable wait(), and it would
then be possible for these threads to wake up, instead of the thread
we were trying to wake up (the one waiting on the job).
It would then get stuck forever.
This bug was introduced in 2df639133b
Also add some debugging code for this kind of failure (disabled)
this is because the JobSystem's queue works as a LIFO, by creating
jobs in reverse (memory) order, we attempt to help streaming to
the d-cache on that threads -- until the point where
jobs are stolen.
we also execute the last job immediately instead of creating a job
for it -- since we're already in a job.
- parallel_for doesn't use recursion anymore to create the "leaf"
jobs, this is now done linearly on N thread (one thread per CPU).
This uses less stack space, and reduces miss-predicted branches.
- remove almost all SYSTRACE calls because they have a huge impact
on things like parallel_for() and are misleading. They can be
enabled again by setting HEAVY_SYSTRACE to true.
- we simplify the waiting code by using only a single
condition variable instead of two.
- wait() now behaves just like a looper, it will process jobs until
the one it's waiting for finishes -- before it could just sit there
(the idea was that the job would finish quickly, but that's not always
the case).
- we also make sure to never call notify_n() when it's not needed.
We track how many waiters we have and use that to decide if we need
to notify().
notify is pretty slow on all architectures, even on linux it's always
a syscall, so it's better to avoid it.
- don't use stand-alone fences, makes things ugly for no real benefit
- refactored the code a bit, hopefully it's more clear.
This reverts a JobSystem optimization that attempted to avoid signaling
a condition when there was no waiters. Unfortunately, there was a
race that caused the the signaling thread to miss that the waiter flag
was set, thus not signaling.
- only signal waitAndRelease() when the corresponding job finishes and
only if there is waitAndRelease() active -- instead of signaling
every time a job ends.
- don't surrender time slice when attempting to steal a job and it fails
as long as some queue has jobs.
- check that we have to wait, because taking the lock
- add a benchmark
This change more than doubles the amount of jobs we can handle per
second (~965,000 jobs/s on Pixel3)
JobSystem::waitAndRelease used to spin to wait for the job to finish,
usually this wasn't a problem because the spinning thread was
able to handle other jobs. However, in cases where no job was
available it would actually spin in burn cpu cycles.
we now use a (separate) condition variable to handle that case.
* Clean-up EntityManager a bit
- use tsl::robin_set instead of std::set (which should have been unordered::set
anyways).
- getListeners() now returns a vector which avoids to traverse a set twice.
Turns out that copying the set wasn't as efficient as I thought.
* Improve jobsystem a bit
We recently added a job reference counting mechanism, but we were a bit
too aggressive about taking/release references.
Also make the API more complete by adding explicit retain/release,
which is needed to allow several threads to wait on the same job.
Also improve futex code by inlining it.
* Minor clean-ups
- fix a couple usage of std::function
- fix a couple usage of std::string
- remove ALIGN_LOOP, which didn't work
- fix a couple explicit/noexcept
- virtual -> override
* Fix spelling typos and other minor clang-tidy
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.
* 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.
* 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.
Mathias Agopian
Philip Rideout
LaiJF
Benjamin Doherty
Pixelflinger
Romain Guy
Romain Guy