This improves performance by **reducing** the amounts of threads used for tasks
which require a high memory bandwidth.
This works because the underlying hardware has a certain maximum memory
bandwidth. If that is used up by a few threads already, any additional threads
wanting to use a lot of memory will just cause more contention which actually
slows things down. By reducing the number of threads that can perform certain
tasks, the remaining threads are also not locked up doing work that they can't
do efficiently. It's best if there is enough scheduled work so that these tasks
can do more compute intensive tasks instead.
To use this new functionality, one has to put the parallel code in question into
a `threading::memory_bandwidth_bound_task(...)` block. Additionally, one also
has to provide a (very) rough approximation for how many bytes are accessed. If
the number is low, the number of threads shouldn't be reduced because it's
likely that all touched memory can be in L3 cache which generally has a much
higher bandwidth than main memory.
The exact number of threads that are allowed to do bandwidth bound tasks at the
same time is generally highly context and hardware dependent. It's also not
really possible to measure reliably because it depends on so many static and
dynamic factors. The thread count is now hardcoded to 8. It seems that this many
threads are easily capable of maxing out the bandwidth capacity.
With this technique I can measure surprisingly good performance improvements:
* Generating a 3000x3000 grid: 133ms -> 103ms.
* Generating a mesh line with 100'000'000 vertices: 212ms -> 189ms.
* Realize mesh instances resulting in ~27'000'000 vertices: 460ms -> 305ms.
In all of these cases, only 8 instead of 24 threads are used. The remaining
threads are idle in these cases, but they could do other work if available.
Pull Request: https://projects.blender.org/blender/blender/pulls/118939
The `IndexMask` data structure was designed to allow us to implement set
operations like `union`, `intersection` and `difference` efficiently
(2cfcb8b0b8). This patch adds an evaluator for
arbitrary expressions involving the mentioned operations. The evaluator makes
use of the design of the `IndexMask` data structure to be quite efficient.
In some common cases, the evaluator runs in constant time. So it's very fast
even if the mask contains many millions of indices. If possible the evaluator
works on entire segments at once instead of looking at the individual indices.
This results in a very low constant factor even if the evaluation time is
linear. If the evaluator has to look at the individual indices to be able to
perform the operation, it can make use of multi-threading.
The evaluation consists of the following steps:
1. A coarse evaluation that looks at entire segments at once.
2. All segments that couldn't be fully evaluated by the coarse evaluation are
evaluated exactly by looking at the actual indices. There are two evaluators
for this case. One that is based on `std::set_union` etc. The other one first
converts the index masks to bit spans, then does bit operations to evaluate
the expression, and then converts the bits back into indices. Depending on
the expression, one or the other can be more efficient.
3. Construct an index mask from the evaluated segments.
Showing the performance of the evaluator is kind of difficult because it highly
depends on the input data. Comparing the performance to something that does not
short-circuit when there are full ranges is meaningless, because one can
construct an example where the new evaluator is arbitrarily faster. I'm still
working on a case where performance can be compared to e.g. using
`std::set_union`. This comparison is only fair when the input data when
constructing a case where the new evaluator can't short-circuit.
One of the main remaining bottlenecks are the calls to `slice_content` on large
index masks. I think the impact of those can still be reduced.
We are not using this evaluator much yet, except through `IndexMask::complement`
calls. I intend to use it when I get to refactoring the field evaluator for
geometry nodes to optimize the evaluation of selections.
Pull Request: https://projects.blender.org/blender/blender/pulls/117805
- Remove the unnecessary `ContainerValue` from the class hierarchy
- Construct `StringValue` with a `std::string` by value to avoid copies
- Remove some indirection by using type names directly instead of aliases
- Use utility methods to lookup/append specific data types for arrays/dicts
- Simplify conversion from unique_ptr to shared_ptr
- Avoid use of `new` and `delete`
- Avoid creating maps of all elements in vector for a single lookup
"Own" (the adjective) cannot be used on its own. It should be combined
with something like "its own", "our own", "her own", or "the object's own".
It also isn't used separately to mean something like "separate".
Also, "its own" is correct instead of "it's own" which is a misues of the verb.
The BLI image interpolation function with clamped boundary returns zero
for out of bound pixels. That's because the neighbour pixel wrapping
condition disregarded the border template argument. To fix this, only
handle that condition if in border mode.
Pull Request: https://projects.blender.org/blender/blender/pulls/119164
* Only works on machines with a Qualcomm Snapdragon 8cx Gen3 or above.
Older generation devices are not and will not be supported due to
some driver issues
* Requires VS2022 for building.
* Uses new MSVC preprocessor for sse2neon compatibility.
* SIMD is not enabled, waiting on conversion of blenlib to C++.
Ref #119126
Pull Request: https://projects.blender.org/blender/blender/pulls/117036
- Pass null instead of an empty string to BKE_tempdir_init
because the string isn't meant to be used.
- Never pass null to BLI_temp_directory_path_copy_if_valid
(the caller must check).
- Additional comments for which checks are performed & why
from discussion about #95411.
- Skip leading forward slashes when setting the temp directory.
- Add a utility function to set the temporary directory
which is used for the user preferences & environment variables.
This issue was raised by #95411 where "//" resolves to "/",
then asserts when passed to Blender's file-system functions.
However the crash referenced in this report looks to be caused
by Collada failing to write to the temporary directory which
can be handled separately.
Ref !118872
The standard `threading::parallel_for` function tries to split the range into
uniformly sized subranges. This is great if each element takes approximately
the same amount of time to compute.
However, there are also situations where the time required to do the work for
a single index differs significantly between different indices. In such a case,
it's better to split the tasks into segments while taking the size of each task into
account.
This patch implements `threading::parallel_for_weighted` which allows passing
in an additional callback that returns the size of each task.
Pull Request: https://projects.blender.org/blender/blender/pulls/118348
Not all filesystems on linux supports the RENAME_NOREPLACE flag.
If we get a EINVAL return value, retry with a non atomic operation.
RENAME_NOREPLACE was introduced in 050d48edfc, so this is a regression
fix as well.
Pull Request: https://projects.blender.org/blender/blender/pulls/118571
Unless you're very familiar with `IndexRange`, it's often hard to know what
e.g. `IndexRange(10, 15)` means. Without more context, one could think
that it means `10-14`, `10-15` or `10-24`. This patch adds named constructors
to `IndexRange` to make the behavior more obvious when writing and when
reading the code. With those one can use `IndexRange::from_begin_end(10, 15)`,
`IndexRange::from_begin_end_inclusive(10, 15)` or `IndexRange::from_begin_size(10, 15)`
respectively. While being a bit more verbose, the explicitness makes code easier to
understand and also allows abstracting away some common index computations.
The old unnamed constructor that takes a begin and size is not removed by this patch,
as that would make the patch significantly bigger. I think it's reasonable to generally
use the named constructors going forward and to change the existing usages of the
old constructor over time.
Pull Request: https://projects.blender.org/blender/blender/pulls/118606
The depsgraph CoW mechanism is a bit of a misnomer. It creates an
evaluated copy for data-blocks regardless of whether the copy will
actually be written to. The point is to have physical separation between
original and evaluated data. This is in contrast to the commonly used
performance improvement of keeping a user count and copying data
implicitly when it needs to be changed. In Blender code we call this
"implicit sharing" instead. Importantly, the dependency graph has no
idea about the _actual_ CoW behavior in Blender.
Renaming this functionality in the despgraph removes some of the
confusion that comes up when talking about this, and will hopefully
make the depsgraph less confusing to understand initially too. Wording
like "the evaluated copy" (as opposed to the original data-block) has
also become common anyway.
Pull Request: https://projects.blender.org/blender/blender/pulls/118338
Span is preferrable since it's agnostic of the source container,
makes it clearer that there is no ownership, is 8 bytes smaller,
and can be passed by value.
The `object_to_world` and `world_to_object` matrices are set during
depsgraph evaluation, calculated from the object's animated location,
rotation, scale, parenting, and constraints. It's confusing and
unnecessary to store them with the original data in DNA.
This commit moves them to `ObjectRuntime` and moves the matrices to
use the C++ `float4x4` type, giving the potential for simplified code
using the C++ abstractions. The matrices are accessible with functions
on `Object` directly since they are used so commonly. Though for write
access, directly using the runtime struct is necessary.
The inverse `world_to_object` matrix is often calculated before it's
used, even though it's calculated as part of depsgraph evaluation.
Long term we might not want to store this in `ObjectRuntime` at all,
and just calculate it on demand. Or at least we should remove the
redundant calculations. That should be done separately though.
Pull Request: https://projects.blender.org/blender/blender/pulls/118210
The convex hull tests included a reference AABB-fitting function for
comparison which was used to validate the optimized implementation.
This wasn't great as it depended on matching exact return values and
didn't test the logic of AABB-fitting worked usefully.
Replace this with a more general test that creates random polygons with
known bounds, apply a random rotation & translation, then use
AABB-fitting to un-rotate the points, passing when the bounds are no
larger than the size of the generated input.
Details:
- Make BLI_convexhull_aabb_fit_hull_2d a static function again as it was
only exposed for tests. Use BLI_convexhull_aabb_fit_points_2d instead.
- Remove brute force reference implementation from tests,
moving this to an assertion within convexhull_2d
(disabled by default since it's quite slow).
The lack of these functions in the "single trivial value" and "sliced
GVArray" implementations caused some code to call fack to the base
class functions. Those are much slower since they involve a virtual
function call per element. For example, this changed the runtime of
creating a new boolean attribute set to "true" on one million faces
from 3.4 ms to 0.35 ms.
Pull Request: https://projects.blender.org/blender/blender/pulls/118161
