Scarfski/filament
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix a couple threading vs. allocations

Browse Source 
- prepareVisibleLights was run on a dedicated thread (via JobSystem), 
  but was using its own local ArenaScope. This is wrong because it
  could reset the root arena at any later point. This is fixed by
  just not using a local ArenaScope.

- related to the above, the root Arena (LinearAllocatorArena) didn't
  use a locked policy, which cause also cause problems since some
  allocations are done off the main thread. We now pre-allocate the one
  buffer we need.

This PR also renames some variable and types to improve readability.
This commit is contained in:
Mathias Agopian
2024-01-29 14:55:25 -08:00
committed by Benjamin Doherty
parent 344c04a2d8
commit 8459cbc91c
14 changed files with 106 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
filament/src/Allocators.h
View File

@@ -54,7 +54,7 @@ using LinearAllocatorArena = utils::Arena<
#endif
using ArenaScope = utils::ArenaScope<LinearAllocatorArena>;
using RootArenaScope = utils::ArenaScope<LinearAllocatorArena>;
} // namespace filament

7
filament/src/Froxelizer.cpp
View File

@@ -168,7 +168,8 @@ void Froxelizer::setProjection(const mat4f& projection,
}
bool Froxelizer::prepare(
FEngine::DriverApi& driverApi, ArenaScope& arena, filament::Viewport const& viewport,
FEngine::DriverApi& driverApi, RootArenaScope& rootArenaScope,
filament::Viewport const& viewport,
const mat4f& projection, float projectionNear, float projectionFar) noexcept {
setViewport(viewport);
setProjection(projection, projectionNear, projectionFar);
@@ -199,12 +200,12 @@ bool Froxelizer::prepare(
// light records per froxel (~256 KiB)
mLightRecords = {
arena.allocate<LightRecord>(getFroxelBufferEntryCount(), CACHELINE_SIZE),
rootArenaScope.allocate<LightRecord>(getFroxelBufferEntryCount(), CACHELINE_SIZE),
getFroxelBufferEntryCount() };
// froxel thread data (~256 KiB)
mFroxelShardedData = {
arena.allocate<FroxelThreadData>(GROUP_COUNT, CACHELINE_SIZE),
rootArenaScope.allocate<FroxelThreadData>(GROUP_COUNT, CACHELINE_SIZE),
uint32_t(GROUP_COUNT)
};

2
filament/src/Froxelizer.h
View File

@@ -110,7 +110,7 @@ public:
*
* return true if updateUniforms() needs to be called
*/
bool prepare(backend::DriverApi& driverApi, ArenaScope& arena, Viewport const& viewport,
bool prepare(backend::DriverApi& driverApi, RootArenaScope& rootArenaScope, Viewport const& viewport,
const math::mat4f& projection, float projectionNear, float projectionFar) noexcept;
Froxel getFroxelAt(size_t x, size_t y, size_t z) const noexcept;

5
filament/src/RenderPass.cpp
View File

@@ -60,10 +60,9 @@ namespace filament {
using namespace backend;
RenderPass::RenderPass(FEngine& engine,
RenderPass::Arena& arena) noexcept
RenderPass::RenderPass(FEngine& engine, RenderPass::Arena& arena) noexcept
: mCommandArena(arena),
mCustomCommands(engine.getPerRenderPassAllocator()) {
mCustomCommands(engine.getPerRenderPassArena()) {
}
RenderPass::RenderPass(RenderPass const& rhs) = default;

12
filament/src/RenderPass.h
View File

@@ -31,14 +31,21 @@
#include <utils/Allocator.h>
#include <utils/Range.h>
#include <utils/Slice.h>
#include <utils/architecture.h>
#include <utils/compiler.h>
#include <utils/debug.h>
#include <math/mathfwd.h>
#include <functional>
#include <limits>
#include <type_traits>
#include <vector>
#include <stddef.h>
#include <stdint.h>
namespace filament {
namespace backend {
@@ -244,7 +251,6 @@ public:
uint32_t skinningOffset = 0; // 4 bytes
uint16_t instanceCount; // 2 bytes [MSb: user]
Variant materialVariant; // 1 byte
// uint8_t reserved[0] = {}; // 0 bytes
static const uint16_t USER_INSTANCE_MASK = 0x8000u;
static const uint16_t INSTANCE_COUNT_MASK = 0x7fffu;
@@ -257,7 +263,7 @@ public:
uint64_t reserved[1] = {}; // 8 bytes
bool operator < (Command const& rhs) const noexcept { return key < rhs.key; }
// placement new declared as "throw" to avoid the compiler's null-check
inline void* operator new (std::size_t, void* ptr) {
inline void* operator new (size_t, void* ptr) {
assert_invariant(ptr);
return ptr;
}
@@ -298,7 +304,7 @@ public:
void setGeometry(FScene::RenderableSoa const& soa, utils::Range<uint32_t> vr,
backend::Handle<backend::HwBufferObject> uboHandle) noexcept;
// specifies camera information (e.g. used for sorting commands)
// Specifies camera information (e.g. used for sorting commands)
void setCamera(const CameraInfo& camera) noexcept;
// flags controlling how commands are generated

2
filament/src/details/Engine.cpp
View File

@@ -198,7 +198,7 @@ FEngine::FEngine(Engine::Builder const& builder) :
mCommandBufferQueue(
builder->mConfig.minCommandBufferSizeMB * MiB,
builder->mConfig.commandBufferSizeMB * MiB),
mPerRenderPassAllocator(
mPerRenderPassArena(
"FEngine::mPerRenderPassAllocator",
builder->mConfig.perRenderPassArenaSizeMB * MiB),
mHeapAllocator("FEngine::mHeapAllocator", AreaPolicy::NullArea{}),

26
filament/src/details/Engine.h
View File

@@ -58,17 +58,6 @@
#include <filament/Texture.h>
#include <filament/VertexBuffer.h>
#if FILAMENT_ENABLE_MATDBG
#include <matdbg/DebugServer.h>
#else
namespace filament {
namespace matdbg {
class DebugServer;
using MaterialKey = uint32_t;
} // namespace matdbg
} // namespace filament
#endif
#include <utils/compiler.h>
#include <utils/Allocator.h>
#include <utils/JobSystem.h>
@@ -78,8 +67,19 @@ using MaterialKey = uint32_t;
#include <memory>
#include <new>
#include <random>
#include <thread>
#include <type_traits>
#include <unordered_map>
#if FILAMENT_ENABLE_MATDBG
#include <matdbg/DebugServer.h>
#else
namespace filament::matdbg {
class DebugServer;
using MaterialKey = uint32_t;
} // namespace filament::matdbg
#endif
namespace filament {
class Renderer;
@@ -142,7 +142,7 @@ public:
// the per-frame Area is used by all Renderer, so they must run in sequence and
// have freed all allocated memory when done. If this needs to change in the future,
// we'll simply have to use separate Areas (for instance).
LinearAllocatorArena& getPerRenderPassAllocator() noexcept { return mPerRenderPassAllocator; }
LinearAllocatorArena& getPerRenderPassArena() noexcept { return mPerRenderPassArena; }
// Material IDs...
uint32_t getMaterialId() const noexcept { return mMaterialId++; }
@@ -508,7 +508,7 @@ private:
uint32_t mFlushCounter = 0;
LinearAllocatorArena mPerRenderPassAllocator;
RootArenaScope::Arena mPerRenderPassArena;
HeapAllocatorArena mHeapAllocator;
utils::JobSystem mJobSystem;

43
filament/src/details/Renderer.cpp
View File

@@ -16,6 +16,9 @@
#include "details/Renderer.h"
#include "Allocators.h"
#include "DebugRegistry.h"
#include "FrameHistory.h"
#include "PostProcessManager.h"
#include "RendererUtils.h"
#include "RenderPass.h"
@@ -28,21 +31,40 @@
#include "details/Texture.h"
#include "details/View.h"
#include <filament/Camera.h>
#include <filament/Fence.h>
#include <filament/Options.h>
#include <filament/Renderer.h>
#include <backend/DriverEnums.h>
#include <backend/DriverApiForward.h>
#include <backend/Handle.h>
#include <backend/PixelBufferDescriptor.h>
#include "fg/FrameGraph.h"
#include "fg/FrameGraphId.h"
#include "fg/FrameGraphResources.h"
#include "fg/FrameGraphTexture.h"
#include <math/vec2.h>
#include <math/vec3.h>
#include <math/mat4.h>
#include <utils/compiler.h>
#include <utils/JobSystem.h>
#include <utils/Log.h>
#include <utils/ostream.h>
#include <utils/Panic.h>
#include <utils/Systrace.h>
#include <utils/vector.h>
#include <utils/debug.h>
#include <chrono>
#include <limits>
#include <utility>
#include <stddef.h>
#include <stdint.h>
// this helps visualize what dynamic-scaling is doing
#define DEBUG_DYNAMIC_SCALING false
@@ -62,8 +84,7 @@ FRenderer::FRenderer(FEngine& engine) :
mHdrQualityMedium(TextureFormat::R11F_G11F_B10F),
mHdrQualityHigh(TextureFormat::RGB16F),
mIsRGB8Supported(false),
mUserEpoch(engine.getEngineEpoch()),
mPerRenderPassArena(engine.getPerRenderPassAllocator())
mUserEpoch(engine.getEngineEpoch())
{
FDebugRegistry& debugRegistry = engine.getDebugRegistry();
debugRegistry.registerProperty("d.renderer.doFrameCapture",
@@ -452,17 +473,17 @@ void FRenderer::render(FView const* view) {
}
void FRenderer::renderInternal(FView const* view) {
// per-renderpass data
ArenaScope rootArena(mPerRenderPassArena);
FEngine& engine = mEngine;
JobSystem& js = engine.getJobSystem();
// per-renderpass data
RootArenaScope rootArenaScope(engine.getPerRenderPassArena());
// create a root job so no other job can escape
JobSystem& js = engine.getJobSystem();
auto *rootJob = js.setRootJob(js.createJob());
// execute the render pass
renderJob(rootArena, const_cast<FView&>(*view));
renderJob(rootArenaScope, const_cast<FView&>(*view));
// make sure to flush the command buffer
engine.flush();
@@ -471,7 +492,7 @@ void FRenderer::renderInternal(FView const* view) {
js.runAndWait(rootJob);
}
void FRenderer::renderJob(ArenaScope& arena, FView& view) {
void FRenderer::renderJob(RootArenaScope& rootArenaScope, FView& view) {
FEngine& engine = mEngine;
JobSystem& js = engine.getJobSystem();
FEngine::DriverApi& driver = engine.getDriverApi();
@@ -636,7 +657,7 @@ void FRenderer::renderJob(ArenaScope& arena, FView& view) {
xvp.bottom = int32_t(guardBand);
}
view.prepare(engine, driver, arena, svp, cameraInfo, getShaderUserTime(), needsAlphaChannel);
view.prepare(engine, driver, rootArenaScope, svp, cameraInfo, getShaderUserTime(), needsAlphaChannel);
view.prepareUpscaler(scale, taaOptions, dsrOptions);
@@ -649,7 +670,7 @@ void FRenderer::renderJob(ArenaScope& arena, FView& view) {
// Allocate some space for our commands in the per-frame Arena, and use that space as
// an Arena for commands. All this space is released when we exit this method.
size_t const perFrameCommandsSize = engine.getPerFrameCommandsSize();
void* const arenaBegin = arena.allocate(perFrameCommandsSize, CACHELINE_SIZE);
void* const arenaBegin = rootArenaScope.allocate(perFrameCommandsSize, CACHELINE_SIZE);
void* const arenaEnd = pointermath::add(arenaBegin, perFrameCommandsSize);
RenderPass::Arena commandArena("Command Arena", { arenaBegin, arenaEnd });

5
filament/src/details/Renderer.h
View File

@@ -163,7 +163,7 @@ private:
}
void renderInternal(FView const* view);
void renderJob(ArenaScope& arena, FView& view);
void renderJob(RootArenaScope& rootArenaScope, FView& view);
// keep a reference to our engine
FEngine& mEngine;
@@ -187,9 +187,6 @@ private:
backend::TargetBufferFlags mClearFlags{};
tsl::robin_set<FRenderTarget*> mPreviousRenderTargets;
std::function<void()> mBeginFrameInternal;
// per-frame arena for this Renderer
LinearAllocatorArena& mPerRenderPassArena;
};
FILAMENT_DOWNCAST(Renderer)

10
filament/src/details/Scene.cpp
View File

@@ -53,7 +53,7 @@ FScene::~FScene() noexcept = default;
void FScene::prepare(utils::JobSystem& js,
LinearAllocatorArena& allocator,
RootArenaScope& rootArenaScope,
mat4 const& worldTransform,
bool shadowReceiversAreCasters) noexcept {
// TODO: can we skip this in most cases? Since we rely on indices staying the same,
@@ -64,7 +64,7 @@ void FScene::prepare(utils::JobSystem& js,
SYSTRACE_CONTEXT();
// This will reset the allocator upon exiting
ArenaScope const arena(allocator);
ArenaScope<RootArenaScope::Arena> localArenaScope(rootArenaScope.getArena());
FEngine& engine = mEngine;
EntityManager const& em = engine.getEntityManager();
@@ -85,10 +85,10 @@ void FScene::prepare(utils::JobSystem& js,
utils::STLAllocator< LightContainerData, LinearAllocatorArena >, false>;
RenderableInstanceContainer renderableInstances{
RenderableInstanceContainer::with_capacity(entities.size(), allocator) };
RenderableInstanceContainer::with_capacity(entities.size(), localArenaScope.getArena()) };
LightInstanceContainer lightInstances{
LightInstanceContainer::with_capacity(entities.size(), allocator) };
LightInstanceContainer::with_capacity(entities.size(), localArenaScope.getArena()) };
SYSTRACE_NAME_BEGIN("InstanceLoop");
@@ -454,7 +454,7 @@ void FScene::terminate(FEngine&) {
mRenderableViewUbh.clear();
}
void FScene::prepareDynamicLights(const CameraInfo& camera, ArenaScope&,
void FScene::prepareDynamicLights(const CameraInfo& camera,
Handle<HwBufferObject> lightUbh) noexcept {
FEngine::DriverApi& driver = mEngine.getDriverApi();
FLightManager const& lcm = mEngine.getLightManager();

6
filament/src/details/Scene.h
View File

@@ -31,6 +31,8 @@
#include <filament/Box.h>
#include <filament/Scene.h>
#include <math/mathfwd.h>
#include <utils/compiler.h>
#include <utils/Entity.h>
#include <utils/Slice.h>
@@ -70,12 +72,12 @@ public:
~FScene() noexcept;
void terminate(FEngine& engine);
void prepare(utils::JobSystem& js, LinearAllocatorArena& allocator,
void prepare(utils::JobSystem& js, RootArenaScope& rootArenaScope,
math::mat4 const& worldTransform, bool shadowReceiversAreCasters) noexcept;
void prepareVisibleRenderables(utils::Range<uint32_t> visibleRenderables) noexcept;
void prepareDynamicLights(const CameraInfo& camera, ArenaScope& arena,
void prepareDynamicLights(const CameraInfo& camera,
backend::Handle<backend::HwBufferObject> lightUbh) noexcept;
backend::Handle<backend::HwBufferObject> getRenderableUBO() const noexcept {

47
filament/src/details/View.cpp
View File

@@ -341,8 +341,7 @@ void FView::prepareShadowing(FEngine& engine, FScene::RenderableSoa& renderableD
mNeedsShadowMap = any(shadowTechnique & ShadowMapManager::ShadowTechnique::SHADOW_MAP);
}
void FView::prepareLighting(FEngine& engine, ArenaScope& arena,
CameraInfo const& cameraInfo) noexcept {
void FView::prepareLighting(FEngine& engine, CameraInfo const& cameraInfo) noexcept {
SYSTRACE_CALL();
SYSTRACE_CONTEXT();
@@ -354,7 +353,7 @@ void FView::prepareLighting(FEngine& engine, ArenaScope& arena,
*/
if (hasDynamicLighting()) {
scene->prepareDynamicLights(cameraInfo, arena, mLightUbh);
scene->prepareDynamicLights(cameraInfo, mLightUbh);
}
// here the array of visible lights has been shrunk to CONFIG_MAX_LIGHT_COUNT
@@ -427,7 +426,7 @@ CameraInfo FView::computeCameraInfo(FEngine& engine) const noexcept {
return { *camera, mat4{ rotation } * mat4::translation(translation) };
}
void FView::prepare(FEngine& engine, DriverApi& driver, ArenaScope& arena,
void FView::prepare(FEngine& engine, DriverApi& driver, RootArenaScope& rootArenaScope,
filament::Viewport viewport, CameraInfo cameraInfo,
float4 const& userTime, bool needsAlphaChannel) noexcept {
@@ -465,7 +464,7 @@ void FView::prepare(FEngine& engine, DriverApi& driver, ArenaScope& arena,
* Gather all information needed to render this scene. Apply the world origin to all
* objects in the scene.
*/
scene->prepare(js, arena.getAllocator(),
scene->prepare(js, rootArenaScope,
cameraInfo.worldTransform,
hasVSM());
@@ -475,14 +474,22 @@ void FView::prepare(FEngine& engine, DriverApi& driver, ArenaScope& arena,
JobSystem::Job* froxelizeLightsJob = nullptr;
JobSystem::Job* prepareVisibleLightsJob = nullptr;
if (scene->getLightData().size() > FScene::DIRECTIONAL_LIGHTS_COUNT) {
size_t const lightCount = scene->getLightData().size();
if (lightCount > FScene::DIRECTIONAL_LIGHTS_COUNT) {
// create and start the prepareVisibleLights job
// note: this job updates LightData (non const)
// allocate a scratch buffer for distances outside the job below, so we don't need
// to use a locked allocator; the downside is that we need to account for the worst case.
size_t const positionalLightCount = lightCount - FScene::DIRECTIONAL_LIGHTS_COUNT;
float* const distances = rootArenaScope.allocate<float>(
(positionalLightCount + 3u) & ~3u, CACHELINE_SIZE);
prepareVisibleLightsJob = js.runAndRetain(js.createJob(nullptr,
[&engine, &arena, &viewMatrix = cameraInfo.view, &cullingFrustum,
[&engine, distances, positionalLightCount, &viewMatrix = cameraInfo.view, &cullingFrustum,
&lightData = scene->getLightData()]
(JobSystem&, JobSystem::Job*) {
FView::prepareVisibleLights(engine.getLightManager(), arena,
FView::prepareVisibleLights(engine.getLightManager(),
{ distances, distances + positionalLightCount },
viewMatrix, cullingFrustum, lightData);
}));
}
@@ -530,7 +537,7 @@ void FView::prepare(FEngine& engine, DriverApi& driver, ArenaScope& arena,
// As soon as prepareVisibleLight finishes, we can kick-off the froxelization
if (hasDynamicLighting()) {
auto& froxelizer = mFroxelizer;
if (froxelizer.prepare(driver, arena, viewport,
if (froxelizer.prepare(driver, rootArenaScope, viewport,
cameraInfo.projection, cameraInfo.zn, cameraInfo.zf)) {
// TODO: might be more consistent to do this in prepareLighting(), but it's not
// strictly necessary
@@ -645,7 +652,7 @@ void FView::prepare(FEngine& engine, DriverApi& driver, ArenaScope& arena,
* Relies on FScene::prepare() and prepareVisibleLights()
*/
prepareLighting(engine, arena, cameraInfo);
prepareLighting(engine, cameraInfo);
/*
* Update driver state
@@ -850,7 +857,8 @@ void FView::cullRenderables(JobSystem&,
functor(0, renderableData.size());
}
void FView::prepareVisibleLights(FLightManager const& lcm, ArenaScope& rootArena,
void FView::prepareVisibleLights(FLightManager const& lcm,
utils::Slice<float> scratch,
mat4f const& viewMatrix, Frustum const& frustum,
FScene::LightSoa& lightData) noexcept {
SYSTRACE_CALL();
@@ -918,28 +926,25 @@ void FView::prepareVisibleLights(FLightManager const& lcm, ArenaScope& rootArena
* - This helps our limited numbers of spot-shadow as well.
*/
ArenaScope arena(rootArena.getAllocator());
size_t const size = visibleLightCount;
// number of point/spotlights
size_t const positionalLightCount = size - FScene::DIRECTIONAL_LIGHTS_COUNT;
size_t const positionalLightCount = visibleLightCount - FScene::DIRECTIONAL_LIGHTS_COUNT;
if (positionalLightCount) {
// always allocate at least 4 entries, because the vectorized loops below rely on that
float* const UTILS_RESTRICT distances =
arena.allocate<float>((size + 3u) & ~3u, CACHELINE_SIZE);
assert_invariant(positionalLightCount <= scratch.size());
// pre-compute the lights' distance to the camera, for sorting below
// - we don't skip the directional light, because we don't care, it's ignored during sorting
float* const UTILS_RESTRICT distances = scratch.data();
float4 const* const UTILS_RESTRICT spheres = lightData.data<FScene::POSITION_RADIUS>();
computeLightCameraDistances(distances, viewMatrix, spheres, size);
computeLightCameraDistances(distances, viewMatrix, spheres, visibleLightCount);
// skip directional light
Zip2Iterator<FScene::LightSoa::iterator, float*> b = { lightData.begin(), distances };
std::sort(b + FScene::DIRECTIONAL_LIGHTS_COUNT, b + size,
std::sort(b + FScene::DIRECTIONAL_LIGHTS_COUNT, b + visibleLightCount,
[](auto const& lhs, auto const& rhs) { return lhs.second < rhs.second; });
}
// drop excess lights
lightData.resize(std::min(size, CONFIG_MAX_LIGHT_COUNT + FScene::DIRECTIONAL_LIGHTS_COUNT));
lightData.resize(std::min(visibleLightCount,
CONFIG_MAX_LIGHT_COUNT + FScene::DIRECTIONAL_LIGHTS_COUNT));
}
// These methods need to exist so clang honors the __restrict__ keyword, which in turn

7
filament/src/details/View.h
View File

@@ -88,7 +88,7 @@ public:
// note: viewport/cameraInfo are passed by value to make it clear that prepare cannot
// keep references on them that would outlive the scope of prepare() (e.g. with JobSystem).
void prepare(FEngine& engine, backend::DriverApi& driver, ArenaScope& arena,
void prepare(FEngine& engine, backend::DriverApi& driver, RootArenaScope& rootArenaScope,
filament::Viewport viewport, CameraInfo cameraInfo,
math::float4 const& userTime, bool needsAlphaChannel) noexcept;
@@ -144,7 +144,7 @@ public:
void prepareShadowing(FEngine& engine, FScene::RenderableSoa& renderableData,
FScene::LightSoa const& lightData, CameraInfo const& cameraInfo) noexcept;
void prepareLighting(FEngine& engine, ArenaScope& arena, CameraInfo const& cameraInfo) noexcept;
void prepareLighting(FEngine& engine, CameraInfo const& cameraInfo) noexcept;
void prepareSSAO(backend::Handle<backend::HwTexture> ssao) const noexcept;
void prepareSSR(backend::Handle<backend::HwTexture> ssr, bool disableSSR,
@@ -460,7 +460,8 @@ private:
void prepareVisibleRenderables(utils::JobSystem& js,
Frustum const& frustum, FScene::RenderableSoa& renderableData) const noexcept;
static void prepareVisibleLights(FLightManager const& lcm, ArenaScope& rootArena,
static void prepareVisibleLights(FLightManager const& lcm,
utils::Slice<float> scratch,
math::mat4f const& viewMatrix, Frustum const& frustum,
FScene::LightSoa& lightData) noexcept;

4
libs/utils/include/utils/Allocator.h
View File

@@ -720,6 +720,8 @@ class ArenaScope {
}
public:
using Arena = ARENA;
explicit ArenaScope(ARENA& allocator)
: mArena(allocator), mRewind(allocator.getCurrent()) {
}
@@ -771,7 +773,7 @@ public:
}
// use with caution
ARENA& getAllocator() noexcept { return mArena; }
ARENA& getArena() noexcept { return mArena; }
private:
ARENA& mArena;