Вторая часть исследования Nau Engine

Во второй части нашей трилогии об игровом движке Nau Engine мы обсудим важные аспекты оптимизации и повышения производительности. Наша цель — выявить проблемы, которые могут повлиять на эффективность и стабильность игр, созданных с использованием Nau Engine.

Вторая часть исследования Nau Engine

В первой части мы сосредоточились на функциональности Nau Engine, разобрав три категории ошибок: проблемы с памятью, копипасту и логические ошибки. Однако, помимо этих аспектов, не менее важную роль играет и производительность. Давайте посмотрим на результаты проверки с помощью PVS-Studio.

Фрагмент N1

std::vector<AnimationTargetData> m_targets; void AnimationComponent::addAnimationTarget(IAnimationTarget::Ptr target) { if (target) { if (auto* nauObject = target->as<scene::NauObject*>()) { .... m_targets.push_back(AnimationTargetData(std::move(wrapper), nullptr)); } else { m_targets.push_back(AnimationTargetData(std::move(target), nullptr)); } } }

Предупреждение PVS-Studio: V823 Decreased performance. Object may be created in-place in the 'm_targets' container. Consider replacing methods: 'push_back' -> 'emplace_back'. animation_component.cpp 180

В коде используется `push_back` для добавления объектов в вектор `m_targets`, что приводит к созданию временного объекта, который затем копируется или перемещается в контейнер. Это приводит к лишнему вызову конструктора перемещения/копирования. Чтобы создать объект непосредственно в контейнере без промежуточных шагов, лучше заменить `push_back` на `emplace_back`. Использование `emplace_back` позволяет передать аргументы конструктора в вектор, создавая объект "по месту" и избегая дополнительных накладных расходов.

Фрагмент N2

void writeContainerHeader(....) { .... const eastl::string contentLength = eastl::to_string(serializedData.size()); Vector<eastl::tuple<eastl::string, eastl::string>> httpHeader = { {"NauContent-Kind", eastl::string(kind)}, {"Content-Type", "application/json"}, {"Content-Length", std::move(contentLength)} }; .... }

Предупреждение PVS-Studio: V833 Passing the const-qualified object 'contentLength' to the 'std::move' function disables move semantics. nau_container.cpp 68

Объект `contentLength` типа `eastl::string` объявлен с квалификатором `const`. Затем разработчик захотел переместить его внутрь вектора `httpHeader` и применил для этого `std::move`. К сожалению, перемещения не произойдёт, так как не будет выбрана перегрузка конструктора `eastl::string` с rvalue-ссылкой. Вместо этого, по правилам выбора перегрузок, предпочтение будет отдано конструктору копирования.

Решение проблемы очень простое: убрать квалификатор `const` у `contentLength`, и тогда объект сможет быть перемещён.

Однако это не всё, в этом коде есть ещё одно неявное копирование строк. При объявлении `httpHeader` вызывается конструктор от `std::initializer_list`. Последний представляет собой легковесный прокси-объект над массивом типа `const T`. В итоге компилятор представит код примерно следующим образом:

void writeContainerHeader(....) { .... eastl::string contentLength = eastl::to_string(serializedData.size()); const eastl::tuple<eastl::string, eastl::string> backing_array[] { {"NauContent-Kind", eastl::string(kind)}, {"Content-Type", "application/json"}, {"Content-Length", std::move(contentLength)} }; Vector<eastl::tuple<eastl::string, eastl::string>> httpHeader { std::initializer_list { backing_array } }; .... }

Исходя из этого константные кортежи будут копироваться в вектор, а это приведёт к копированию строк внутри константного массива. Избежать этого можно, если отказаться от std::initializer_list в пользу вызовов emplace_back:

void writeContainerHeader(....) { .... eastl::string contentLength = eastl::to_string(serializedData.size()); Vector<eastl::tuple<eastl::string, eastl::string>> httpHeader; httpHeader.reserve(3); httpHeader.emplace_back("NauContent-Kind", eastl::string(kind)); httpHeader.emplace_back("Content-Type", "application/json"); httpHeader.emplace_back("Content-Length", std::move(contentLength)); }

Такой код будет более производительным, но ценой лаконичности.

Фрагмент N3

inline const ComponentInfo getEntityComponentInfo(....) const { EntityComponentRef ref = getEntityComponentRef(eid, cid); if (ref.isNull()) return ComponentInfo("<invalid>", eastl::move(ref)); return ComponentInfo(dataComponents .getComponentNameById(ref.getComponentId()), eastl::move(ref)); }

Предупреждение PVS-Studio: V839 The 'EntityManager::getEntityComponentInfo' function returns a constant value. This may interfere with move semantics. entityManager.h 1855

Функция `getEntityComponentInfo` возвращает объекты типа `const ComponentInfo`. Такое написание возвращаемого типа считается устаревшим (CppCoreGuidelines F.49), и до C++17 может привести к лишнему копированию, когда объект инициализируется результатом вызова функции.

Решение проблемы простое: убрать `const` из возвращаемого типа.

inline ComponentInfo getEntityComponentInfo(....) const { .... }

И вот ещё случаи:

  • V839 The 'getCommit' function returns a constant value. This may interfere with move semantics. engine_version.h 54
  • V839 The 'getBranch' function returns a constant value. This may interfere with move semantics. engine_version.h 59
  • V839 The 'getFullPathCache' function returns a constant value. This may interfere with move semantics. CCFileUtils.h 831
  • V839 The 'FileUtils::getSearchResolutionsOrder' function returns a constant value. This may interfere with move semantics. CCFileUtils.cpp 971
  • V839 The 'FileUtils::getSearchPaths' function returns a constant value. This may interfere with move semantics. CCFileUtils.cpp 977
  • V839 The 'FileUtils::getOriginalSearchPaths' function returns a constant value. This may interfere with move semantics. CCFileUtils.cpp 983
  • V839 The 'FileUtils::getDefaultResourceRootPath' function returns a constant value. This may interfere with move semantics. CCFileUtils.cpp 995
  • V839 The 'ProgramNau::getActiveAttributes' function returns a constant value. This may interfere with move semantics. program_nau.cpp 99

Фрагмент N4

eastl::unique_ptr<uint8_t[]> convertData(....) { switch (format) { case cocos2d::backend::PixelFormat::RGBA4444: { .... eastl::unique_ptr<uint8_t[]> newData{ new uint8_t[....] }; .... return std::move(newData); } .... }

Предупреждение PVS-Studio: V828 Decreased performance. Moving a local object in a return statement prevents copy elision. texture_nau.cpp 78

В этом фрагменте кода мы имеем дело с функцией, которая возвращает объект типа `eastl::unique_ptr`. Когда возвращаемый тип функции совпадает с типом возвращаемого значения, и это значение является локальной переменной, компилятор может выполнить одну из следующих операций:

  • Named Return Value Optimization (NRVO) — объект может быть создан непосредственно в месте вызова функции, что позволяет избежать любых перемещений или копирований;

  • применение конструктора перемещения — если NRVO не может быть применён, компилятор может использовать перемещение, чтобы передать владение объектом;

  • применение конструктора копирования — в случае, если ни NRVO, ни перемещение не могут быть использованы, будет применен конструктор копирования.

Когда мы применяем std::move, выражение в return становится отличным от возвращаемого типа функции, что предотвращает возможность применения NRVO. В результате код оказывается менее эффективным, так как компилятор может использовать либо перемещение, либо копирование вместо более оптимального NRVO.

Таким образом, выражение std::move(newData) ведёт к пессимизации, а поэтому вызов функции std::move стоит удалить.

Фрагмент N5

void configureVirtualFileSystem() { .... fs::path currentPath = fs::current_path(); auto& props = getServiceProvider().get<GlobalProperties>(); if (auto contentPath = props.getValue<eastl::string>("contentPath"); contentPath) { const std::filesystem::path path = contentPath->c_str(); #ifdef NAU_PACKAGE_BUILD for (const auto& entry : fs::directory_iterator(path)) { if ( entry.is_regular_file() && entry.path().extension() == ".assets") { const auto& filePath = entry.path(); auto assetPackFS = io::createAssetPackFileSystem( strings::toU8StringView(filePath.string()) ); vfs.mount("/packs", assetPackFS).ignore(); assetDb.addAssetDB("packs/assets_database/database.db"); } } #else auto contentFs = io::createNativeFileSystem(path.string()); vfs.mount("/content", std::move(contentFs)).ignore(); auto assetDbPath = path.parent_path() / "assets_database"; vfs.mount("/assets_db", std::move(contentFs)).ignore(); assetDb.addAssetDB("assets_db/database.db"); #endif } }

Предупреждение PVS-Studio: V808 'currentPath' object of 'path' type was created but was not utilized. default_application_delegate.cpp 101

Анализатор обнаружил, что переменная `currentPath`, созданная для хранения текущего пути с помощью `fs::current_path()`, не используется в дальнейшем коде функции `configureVirtualFileSystem()`. Возможно, что после рефакторинга кода локальная переменная перестала использоваться, и её можно удалить. Это никак не повлияет на логику функции.

Фрагмент N6

class ThreadPoolExecutor final : public Executor, public IRuntimeComponent { public: ThreadPoolExecutor(std::optional<size_t> threadsCount) { const size_t maxThreads = threadsCount ? *threadsCount : getDefaultThreadsCount(); m_threads.reserve(maxThreads); for (size_t i = 0; i < maxThreads; ++i) { m_threads.emplace_back([](ThreadPoolExecutor& executor, size_t threadIndex) { threading::setThisThreadName( std::format("Nau Pool-{}", threadIndex + 1) ); executor.threadWork(); }, std::ref(*this), i); } RuntimeObjectRegistration{nau::Ptr<>{this}}.setAutoRemove(); void(); } .... };

Предупреждение PVS-Studio: V607 Ownerless expression 'void ()'. thread_pool_executor.cpp 60

В конструкторе класса `ThreadPoolExecutor` создаются рабочие потоки. И в конце, как вишенка на торте, тело конструктора украшает конструкция `void();`, которая не выполняет никакой полезной работы. Трудно сказать, почему она там появилась. Возможно, в результате неаккуратного рефакторинга, или на месте этой конструкции должно быть что-то другое.

Фрагмент N7

struct ScheduledArchetypeComponentTrack { .... ScheduledArchetypeComponentTrack() {} .... };

Предупреждение PVS-Studio: V832 It's better to use '= default;' syntax instead of empty constructor body. ecsQueryInternal.h 35

Структура `ScheduledArchetypeComponentTrack` имеет определённый пользователем конструктор. Однако лучше объявить его по умолчанию: в таком случае класс будет тривиально конструируемым. На основании этого компилятор может генерировать более оптимизированный код. Более того, некоторые алгоритмы могут выбирать (бенчмарк) другую, более быструю стратегию при работе с тривиально конструируемыми объектами по умолчанию.

Улучшенный код:

struct ScheduledArchetypeComponentTrack { .... ScheduledArchetypeComponentTrack() = default; .... };

И вот ещё случаи:

Фрагмент N8

if (!this->hasComponent(depConstString.hash) && (!optional || (optional && !can_skip_optional))) { .... }

Предупреждение PVS-Studio: V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!optional' and 'optional'. dataComponent.cpp 283

Выражение `(!optional || (optional && !can_skip_optional))` может быть упрощено. Правая часть оператора `||` будет вычисляться лишь в том случае, если `optional` конвертируется в значение `true`. Если это так, то левый операнд оператора `&&` всегда будет `true`. Следовательно, проверка излишняя, и код можно упростить до следующего вида для повышения читабельности:

if (!this->hasComponent(depConstString.hash) && (!optional || !can_skip_optional)) { .... }

Фрагмент N9

// performance_profiling.h static const nau::PerfTagFlag NAU_PERFTAGS = ....;

Предупреждение PVS-Studio: V1043 A global object variable 'NAU_PERFTAGS' is declared in the header. Multiple copies of it will be created in all translation units that include this header file. performance_profiling.h 23

Объявление констант в заголовочном файле — нормальная операция на первый взгляд. Однако дьявол кроется в деталях. В C++ объекты, объявленные как `const` в пространстве имён (в том числе глобальном), имеют внутреннее связывание. Когда заголовочный файл включается в несколько единиц трансляции, каждый из них будет иметь свою собственную копию этой константы, что может привести к увеличению размера исполняемого файла.

Чтобы избежать проблем, можно использовать один из следующих подходов.

До C++17. Нужно разбить объявление и определение этой константы. Объявление константы производим в заголовочном файле, воспользовавшись спецификатором `extern`. Фактическое определение константы переносим в файл реализации:

// performance_profiling.h extern const nau::PerfTagFlag NAU_PERFTAGS; // Объявление // performance_profiling.cpp const nau::PerfTagFlag NAU_PERFTAGS = ....; // Определение

Начиная с C++17. Объявить константу в заголовочном файле со спецификатором inline. Таким образом, в программе будет существовать лишь одна версия этой константы:

// performance_profiling.h inline static const nau::PerfTagFlag NAU_PERFTAGS = ...;

Фрагмент N10

std::string Paths::getAssetsPath() const { if (m_paths.find("assets") == m_paths.end()) { return ""; } return m_paths.find("assets")->second; }

Предупреждение PVS-Studio: V838 Temporary object is constructed during the call of the 'find' function. Consider using an ordered associative container with heterogeneous lookup to avoid construction of temporary objects. file_system.cpp 421

Давайте для начала взглянем, как объявлено поле `Paths::m_paths`:

class SHARED_API Paths { .... private: .... std::map<std::string, std::string> m_paths; };

Итак, поле `Paths::m_paths` — это ассоциативный сортированный контейнер. Его функция-член `std::map::find` принимает объект того же типа, что и ключ. Это значит, что строковый литерал будет конвертироваться в `std::string`, а это может повлечь за собой динамическую аллокацию.

Такой поиск называется гомогенным, то есть когда передаваемый тип и ключ внутри контейнера совпадают. Начиная с C++14, для ассоциативных сортированных контейнеров добавлен гетерогенный поиск, то есть передаваемый тип и ключ внутри контейнера могут не совпадать. Это может давать прирост производительности, так как не приходится производить дополнительную конвертацию.

Чтобы активировать гетерогенный поиск, нужно передать в `std::map` в качестве третьего шаблонного аргумента тип `std::less<>`:

class SHARED_API Paths { .... private: .... std::map<std::string, std::string, std::less<>> m_paths; };

Помимо этой оптимизации, можно также заметить, что объект по ключу "assets" ищут дважды по контейнеру, что выглядит весьма неоптимально. Поэтому я бы предложил следующее исправление:

std::string Paths::getAssetsPath() const { auto it = m_paths.find("assets"); if (it == m_paths.end()) return {}; return it->second; }

Фрагмент N11

class DeferredRT { .... protected: .... StereoMode m_stereoMode = StereoMode::MonoOrMultipass; int m_numRt = 0, m_width = 0, m_height = 0; nau::string m_name; d3d::SamplerHandle m_defaultSampler; ResizableResPtrTex m_mrts[MAX_NUM_MRT] = {}; ResizableResPtrTex m_depth; bool m_useResolvedDepth = false; }; DeferredRT::DeferredRT(const char* name, int w, int h, StereoMode stereoMode, unsigned msaaFlag, int numRT, const unsigned texFmt[MAX_NUM_MRT], uint32_t depthFmt) : m_stereoMode(stereoMode) { m_name = name; .... }

Предупреждение PVS-Studio: V818 It is more efficient to use an initialization list 'm_name(name)' rather than an assignment operator. deferredRT.cpp 117

Что мы видим здесь: в классе `DeferredRT` поле `m_name` инициализируется в теле конструктора. Однако делается это не самым оптимальным способом. Прежде, чем поток управления попадёт в тело конструктора, исполняется список инициализации. Если поле не указано в нём, то оно будет инициализировано по умолчанию. Когда строка конструируется по умолчанию, наиболее часто происходит следующее:

  • ставится пометка, что объект не содержит динамической аллокации (Small String Optimization);

Затем в теле конструктора вызывается оператор `=`, который отбрасывает результат инициализации по умолчанию. Чтобы исправить положение, достаточно проинициализировать поле в списке инициализации конструктора:

DeferredRT::DeferredRT(const char* name, int w, int h, StereoMode stereoMode, unsigned msaaFlag, int numRT, const unsigned texFmt[MAX_NUM_MRT], uint32_t depthFmt) : m_stereoMode(stereoMode) , m_name(name) { .... }

И вот ещё случаи:

  • V818 It is more efficient to use an initialization list 'm_stream(stream)' rather than an assignment operator. nanim_asset_container.cpp 29
  • V818 It is more efficient to use an initialization list 'm_stream(stream)' rather than an assignment operator. material_asset_container.cpp 51
  • V818 It is more efficient to use an initialization list 'm_stream(shaderPackStream)' rather than an assignment operator. shader_asset_container.cpp 60
  • V818 It is more efficient to use an initialization list 'c(p)' rather than an assignment operator. dag_bounds3.h 209

Заключение

Мы рассмотрели основные методы, которые помогут разработчикам улучшить производительность движка Nau Engine. Понимание и устранение этих проблем — важный шаг на пути к созданию успешных и увлекательных игр. В следующей статье мы поговорим о распространённых ошибках при написании классов.

Спасибо за внимание!

5
4 комментария