CommandStream shouldn't be copyable.

This actually avoids a copy of Dispatcher (which is pretty large)
when constructing Engine.
This commit is contained in:
Mathias Agopian
2022-04-07 14:07:20 -07:00
committed by Mathias Agopian
parent 5f7967d09c
commit 35753bc6c2
7 changed files with 47 additions and 28 deletions
@@ -64,8 +64,10 @@ public:
// executes this command and returns the next one
inline CommandBase* execute(Driver& driver) {
// it is important to return the next command offset by output parameter instead
// of return value -- it allows the compiler to perform the tail call optimization.
// returning the next command by output parameter allows the compiler to perform the
// tail-call optimization in the function called by mExecute, however that comes at
// a cost here (writing and reading the stack at each iteration), in the end it's
// probably better to pay the cost at just one location.
intptr_t next;
mExecute(driver, this, &next);
return reinterpret_cast<CommandBase*>(reinterpret_cast<intptr_t>(this) + next);
@@ -184,8 +186,8 @@ public:
#if !defined(NDEBUG) || (FILAMENT_DEBUG_COMMANDS >= FILAMENT_DEBUG_COMMANDS_ENABLE)
// For now, simply pass the method name down as a string and throw away the parameters.
// This is good enough for certain debugging needs and we can improve this later.
#define DEBUG_COMMAND_BEGIN(methodName, sync, ...) mDriver->debugCommandBegin(this, sync, #methodName)
#define DEBUG_COMMAND_END(methodName, sync) mDriver->debugCommandEnd(this, sync, #methodName)
#define DEBUG_COMMAND_BEGIN(methodName, sync, ...) mDriver.debugCommandBegin(this, sync, #methodName)
#define DEBUG_COMMAND_END(methodName, sync) mDriver.debugCommandEnd(this, sync, #methodName)
#else
#define DEBUG_COMMAND_BEGIN(methodName, sync, ...)
#define DEBUG_COMMAND_END(methodName, sync)
@@ -200,9 +202,11 @@ class CommandStream {
};
public:
CommandStream() noexcept = default;
CommandStream(Driver& driver, CircularBuffer& buffer) noexcept;
CommandStream(CommandStream const& rhs) noexcept = delete;
CommandStream& operator=(CommandStream const& rhs) noexcept = delete;
public:
#define DECL_DRIVER_API(methodName, paramsDecl, params) \
inline void methodName(paramsDecl) { \
@@ -219,13 +223,13 @@ public:
AutoExecute callOnExit([=](){ \
DEBUG_COMMAND_END(methodName, true); \
}); \
return apply(&Driver::methodName, *mDriver, std::forward_as_tuple(params)); \
return apply(&Driver::methodName, mDriver, std::forward_as_tuple(params)); \
}
#define DECL_DRIVER_API_RETURN(RetType, methodName, paramsDecl, params) \
inline RetType methodName(paramsDecl) { \
DEBUG_COMMAND_BEGIN(methodName, false, params); \
RetType result = mDriver->methodName##S(); \
RetType result = mDriver.methodName##S(); \
using Cmd = COMMAND_TYPE(methodName##R); \
void* const p = allocateCommand(CommandBase::align(sizeof(Cmd))); \
new(p) Cmd(mDispatcher.methodName##_, RetType(result), APPLY(std::move, params)); \
@@ -271,14 +275,14 @@ public:
private:
inline void* allocateCommand(size_t size) {
assert_invariant(utils::ThreadUtils::isThisThread(mThreadId));
return mCurrentBuffer->allocate(size);
return mCurrentBuffer.allocate(size);
}
// We use a copy of Dispatcher (instead of a pointer) because this removes one dereference
// when executing driver commands.
Driver& UTILS_RESTRICT mDriver;
CircularBuffer& UTILS_RESTRICT mCurrentBuffer;
Dispatcher mDispatcher;
Driver* UTILS_RESTRICT mDriver = nullptr;
CircularBuffer* UTILS_RESTRICT mCurrentBuffer = nullptr;
#ifndef NDEBUG
// just for debugging...
+4 -4
View File
@@ -33,8 +33,7 @@
using namespace utils;
namespace filament {
namespace backend {
namespace filament::backend {
CircularBuffer::CircularBuffer(size_t size) {
mData = alloc(size);
@@ -56,6 +55,7 @@ CircularBuffer::~CircularBuffer() noexcept {
// If the system does not support mmap, emulate soft circular buffer with two buffers next
// to each others and a special case in circularize()
UTILS_NOINLINE
void* CircularBuffer::alloc(size_t size) noexcept {
#if HAS_MMAP
void* data = nullptr;
@@ -127,6 +127,7 @@ void* CircularBuffer::alloc(size_t size) noexcept {
#endif
}
UTILS_NOINLINE
void CircularBuffer::dealloc() noexcept {
#if HAS_MMAP
if (mData) {
@@ -162,5 +163,4 @@ void CircularBuffer::circularize() noexcept {
mTail = mHead;
}
} // namespace backend
} // namespace filament
} // namespace filament::backend
+5 -5
View File
@@ -56,9 +56,9 @@ static UTILS_NOINLINE UTILS_UNUSED std::string extractMethodName(std::string& co
// ------------------------------------------------------------------------------------------------
CommandStream::CommandStream(Driver& driver, CircularBuffer& buffer) noexcept
: mDispatcher(driver.getDispatcher()),
mDriver(&driver),
mCurrentBuffer(&buffer)
: mDriver(driver),
mCurrentBuffer(buffer),
mDispatcher(driver.getDispatcher())
#ifndef NDEBUG
, mThreadId(ThreadUtils::getThreadId())
#endif
@@ -83,8 +83,8 @@ void CommandStream::execute(void* buffer) {
}
}
mDriver->execute([this, buffer]() {
Driver& UTILS_RESTRICT driver = *mDriver;
mDriver.execute([this, buffer]() {
Driver& UTILS_RESTRICT driver = mDriver;
CommandBase* UTILS_RESTRICT base = static_cast<CommandBase*>(buffer);
while (UTILS_LIKELY(base)) {
base = base->execute(driver);
+3 -3
View File
@@ -59,7 +59,7 @@ void BackendTest::initializeDriver() {
DefaultPlatform* platform = DefaultPlatform::create(&backend);
assert_invariant(static_cast<uint8_t>(backend) == static_cast<uint8_t>(sBackend));
driver = platform->createDriver(nullptr);
commandStream = CommandStream(*driver, commandBufferQueue.getCircularBuffer());
commandStream = std::make_unique<CommandStream>(*driver, commandBufferQueue.getCircularBuffer());
}
void BackendTest::executeCommands() {
@@ -67,7 +67,7 @@ void BackendTest::executeCommands() {
auto buffers = commandBufferQueue.waitForCommands();
for (auto& item : buffers) {
if (UTILS_LIKELY(item.begin)) {
commandStream.execute(item.begin);
getDriverApi().execute(item.begin);
commandBufferQueue.releaseBuffer(item);
}
}
@@ -84,7 +84,7 @@ void BackendTest::flushAndWait(uint64_t timeout) {
Handle<HwSwapChain> BackendTest::createSwapChain() {
const NativeView& view = getNativeView();
return commandStream.createSwapChain(view.ptr, 0);
return getDriverApi().createSwapChain(view.ptr, 0);
}
void BackendTest::fullViewport(RenderPassParams& params) {
+2 -2
View File
@@ -58,14 +58,14 @@ protected:
void readPixelsAndAssertHash(const char* testName, size_t width, size_t height,
filament::backend::Handle<filament::backend::HwRenderTarget> rt, uint32_t expectedHash);
filament::backend::DriverApi& getDriverApi() { return commandStream; }
filament::backend::DriverApi& getDriverApi() { return *commandStream; }
filament::backend::Driver& getDriver() { return *driver; }
private:
filament::backend::Driver* driver = nullptr;
filament::backend::CommandBufferQueue commandBufferQueue;
filament::backend::DriverApi commandStream;
std::unique_ptr<filament::backend::DriverApi> commandStream;
filament::backend::Handle<filament::backend::HwBufferObject> uniform;
};
+10 -2
View File
@@ -208,7 +208,9 @@ void FEngine::init() {
SYSTRACE_CALL();
// this must be first.
mCommandStream = CommandStream(*mDriver, mCommandBufferQueue.getCircularBuffer());
assert_invariant( intptr_t(&mDriverApiStorage) % alignof(DriverApi) == 0 );
::new(&mDriverApiStorage) DriverApi(*mDriver, mCommandBufferQueue.getCircularBuffer());
DriverApi& driverApi = getDriverApi();
mResourceAllocator = new ResourceAllocator(driverApi);
@@ -338,6 +340,12 @@ FEngine::~FEngine() noexcept {
SYSTRACE_CALL();
ASSERT_DESTRUCTOR(mTerminated, "Engine destroyed but not terminated!");
// we use mResourceAllocator as a proxy for knowing if init() was called
if (mResourceAllocator) {
std::destroy_at(std::launder(reinterpret_cast<DriverApi*>(&mDriverApiStorage)));
}
delete mResourceAllocator;
delete mDriver;
if (mOwnPlatform) {
@@ -1000,7 +1008,7 @@ bool FEngine::execute() {
// execute all command buffers
for (auto& item : buffers) {
if (UTILS_LIKELY(item.begin)) {
mCommandStream.execute(item.begin);
getDriverApi().execute(item.begin);
mCommandBufferQueue.releaseBuffer(item);
}
}
+9 -2
View File
@@ -76,6 +76,7 @@ using MaterialKey = uint32_t;
#include <chrono>
#include <memory>
#include <new>
#include <random>
#include <unordered_map>
@@ -147,7 +148,11 @@ public:
~FEngine() noexcept;
backend::Driver& getDriver() const noexcept { return *mDriver; }
DriverApi& getDriverApi() noexcept { return mCommandStream; }
DriverApi& getDriverApi() noexcept {
return *std::launder(reinterpret_cast<DriverApi*>(&mDriverApiStorage));
}
DFG* getDFG() const noexcept { return mDFG.get(); }
// the per-frame Area is used by all Renderer, so they must run in sequence and
@@ -423,7 +428,9 @@ private:
std::thread mDriverThread;
backend::CommandBufferQueue mCommandBufferQueue;
DriverApi mCommandStream;
std::aligned_storage<sizeof(DriverApi), alignof(DriverApi)>::type mDriverApiStorage;
static_assert( sizeof(mDriverApiStorage) >= sizeof(DriverApi) );
uint32_t mFlushCounter = 0;
LinearAllocatorArena mPerRenderPassAllocator;