From 6f8b67bbe1b04d1797cb763b2c8b3ce27b28779a Mon Sep 17 00:00:00 2001 From: alandefreitas Date: Fri, 22 Nov 2024 16:16:52 -0300 Subject: [PATCH 1/2] perf(Builder): render templates directly to ostream --- src/lib/Gen/hbs/Builder.cpp | 65 +++++--------- src/lib/Gen/hbs/Builder.hpp | 12 ++- src/lib/Gen/hbs/MultiPageVisitor.cpp | 123 ++++++++++++++------------ src/lib/Gen/hbs/MultiPageVisitor.hpp | 16 +--- src/lib/Gen/hbs/SinglePageVisitor.cpp | 108 ++++++++++++++-------- src/lib/Gen/hbs/SinglePageVisitor.hpp | 19 ++-- 6 files changed, 176 insertions(+), 167 deletions(-) diff --git a/src/lib/Gen/hbs/Builder.cpp b/src/lib/Gen/hbs/Builder.cpp index 45cfd73a98..d3fbdba576 100644 --- a/src/lib/Gen/hbs/Builder.cpp +++ b/src/lib/Gen/hbs/Builder.cpp @@ -10,11 +10,9 @@ // #include "Builder.hpp" -#include "lib/Support/Radix.hpp" #include #include #include -#include #include #include #include @@ -149,9 +147,10 @@ Builder( //------------------------------------------------ -Expected +Expected Builder:: callTemplate( + std::ostream& os, std::string_view name, dom::Value const& context) { @@ -159,13 +158,14 @@ callTemplate( MRDOCS_TRY(auto fileText, files::getFileText(pathName)); HandlebarsOptions options; options.escapeFunction = escapeFn_; - Expected exp = - hbs_.try_render(fileText, context, options); + OutputRef out(os); + Expected exp = + hbs_.try_render_to(out, fileText, context, options); if (!exp) { return Unexpected(Error(exp.error().what())); } - return *exp; + return {}; } //------------------------------------------------ @@ -215,11 +215,15 @@ createContext( } template -Expected +requires std::derived_from || std::same_as +Expected Builder:: -operator()(T const& I) +operator()(std::ostream& os, T const& I) { - auto const templateFile = fmt::format("index.{}.hbs", domCorpus.fileExtension); + std::string const templateFile = + std::derived_from ? + fmt::format("index.{}.hbs", domCorpus.fileExtension) : + fmt::format("index-overload-set.{}.hbs", domCorpus.fileExtension); dom::Object ctx = createContext(I); auto& config = domCorpus->config; @@ -227,44 +231,19 @@ operator()(T const& I) if (config->embedded || isSinglePage) { - return callTemplate(templateFile, ctx); + return callTemplate(os, templateFile, ctx); } auto const wrapperFile = fmt::format("wrapper.{}.hbs", domCorpus.fileExtension); dom::Object wrapperCtx = createFrame(ctx); - wrapperCtx.set("contents", dom::makeInvocable([this, &I, templateFile]( + wrapperCtx.set("contents", dom::makeInvocable([this, &I, templateFile, &os]( dom::Value const& options) -> Expected { // Helper to write contents directly to stream - return callTemplate(templateFile, createContext(I)); - })); - return callTemplate(wrapperFile, wrapperCtx); -} - -Expected -Builder:: -operator()(OverloadSet const& OS) -{ - auto const templateFile = fmt::format("index-overload-set.{}.hbs", domCorpus.fileExtension); - dom::Object ctx = createContext(OS); - - auto& config = domCorpus->config; - bool isSinglePage = !config->multipage; - if (config->embedded || - isSinglePage) - { - return callTemplate(templateFile, ctx); - } - - auto const wrapperFile = fmt::format("wrapper.{}.hbs", domCorpus.fileExtension); - dom::Object wrapperCtx = createFrame(ctx); - wrapperCtx.set("contents", dom::makeInvocable([this, &OS, templateFile]( - dom::Value const& options) -> Expected - { - // Helper to write contents directly to stream - return callTemplate(templateFile, createContext(OS)); + MRDOCS_TRY(callTemplate(os, templateFile, createContext(I))); + return {}; })); - return callTemplate(wrapperFile, wrapperCtx); + return callTemplate(os, wrapperFile, wrapperCtx); } Expected @@ -352,14 +331,12 @@ commonTemplatesDir(std::string_view subdir) const subdir); } - // Define Builder::operator() for each Info type -#define DEFINE(T) template Expected \ - Builder::operator()(T const&) - -#define INFO(Type) DEFINE(Type##Info); +#define INFO(T) template Expected Builder::operator()(std::ostream&, T##Info const&); #include +template Expected Builder::operator()(std::ostream&, OverloadSet const&); + } // hbs } // mrdocs } // clang diff --git a/src/lib/Gen/hbs/Builder.hpp b/src/lib/Gen/hbs/Builder.hpp index 182861c741..bb3f198135 100644 --- a/src/lib/Gen/hbs/Builder.hpp +++ b/src/lib/Gen/hbs/Builder.hpp @@ -57,12 +57,9 @@ class Builder with the index template as the contents. */ template - Expected - operator()(T const&); - - /// @copydoc operator()(T const&) - Expected - operator()(OverloadSet const&); + requires std::derived_from || std::same_as + Expected + operator()(std::ostream& os, T const&); /** Render the contents in the wrapper layout. @@ -123,8 +120,9 @@ class Builder /** Render a Handlebars template from the templates directory. */ - Expected + Expected callTemplate( + std::ostream& os, std::string_view name, dom::Value const& context); diff --git a/src/lib/Gen/hbs/MultiPageVisitor.cpp b/src/lib/Gen/hbs/MultiPageVisitor.cpp index 98f239311e..ca521fcce2 100644 --- a/src/lib/Gen/hbs/MultiPageVisitor.cpp +++ b/src/lib/Gen/hbs/MultiPageVisitor.cpp @@ -17,77 +17,90 @@ namespace clang { namespace mrdocs { namespace hbs { +template +requires std::derived_from || std::same_as void MultiPageVisitor:: -writePage( - std::string_view text, - std::string_view filename) +operator()(T const& I0) { - std::string path = files::appendPath(outputPath_, filename); - std::string dir = files::getParentDir(path); - auto exp = files::createDirectory(dir); - if (!exp) - { - exp.error().Throw(); - } - std::ofstream os; - try + // If T is an OverloadSet, we make a copy for the lambda because + // these are temporary objects that don't live in the corpus. + // Otherwise, the lambda will capture a reference to the corpus Info. + auto Ref = [&I0] { + if constexpr (std::derived_from) + { + return std::ref(I0); + } + else if constexpr (std::same_as) + { + return OverloadSet(I0); + } + }(); + ex_.async([this, Ref](Builder& builder) { - os.open(path, - std::ios_base::binary | - std::ios_base::out | - std::ios_base::trunc // | std::ios_base::noreplace + T const& I = Ref; + + // =================================== + // Open the output file + // =================================== + std::string path = files::appendPath(outputPath_, builder.domCorpus.getXref(I)); + std::string dir = files::getParentDir(path); + if (auto exp = files::createDirectory(dir); !exp) + { + exp.error().Throw(); + } + std::ofstream os; + try + { + os.open(path, + std::ios_base::binary | + std::ios_base::out | + std::ios_base::trunc // | std::ios_base::noreplace ); - os.write(text.data(), static_cast(text.size())); - } - catch(std::exception const& ex) - { - formatError(R"(std::ofstream("{}") threw "{}")", path, ex.what()).Throw(); - } -} + if (!os.is_open()) { + formatError(R"(std::ofstream("{}") failed)", path) + .Throw(); + } + } + catch (std::exception const& ex) + { + formatError(R"(std::ofstream("{}") threw "{}")", path, ex.what()) + .Throw(); + } -template T> -void -MultiPageVisitor:: -operator()(T const& I) -{ - ex_.async([this, &I](Builder& builder) - { - if(const auto r = builder(I)) - writePage(*r, builder.domCorpus.getXref(I)); - else - r.error().Throw(); - if constexpr( + // =================================== + // Generate the output + // =================================== + if (auto exp = builder(os, I); !exp) + { + exp.error().Throw(); + } + + // =================================== + // Traverse the symbol members + // =================================== + if constexpr (std::derived_from) + { + if constexpr( T::isNamespace() || T::isRecord() || T::isEnum()) + { + corpus_.traverseOverloads(I, *this); + } + } + else if constexpr (std::same_as) { - // corpus_.traverse(I, *this); - corpus_.traverseOverloads(I, *this); + corpus_.traverse(I, *this); } }); } -void -MultiPageVisitor:: -operator()(OverloadSet const& OS) -{ - ex_.async([this, OS](Builder& builder) - { - if(const auto r = builder(OS)) - writePage(*r, builder.domCorpus.getXref(OS)); - else - r.error().Throw(); - corpus_.traverse(OS, *this); - }); -} - -#define DEFINE(T) template void \ - MultiPageVisitor::operator()(T const&) - -#define INFO(Type) DEFINE(Type##Info); +#define INFO(T) template void MultiPageVisitor::operator()(T##Info const&); #include +template void MultiPageVisitor::operator()(OverloadSet const&); + } // hbs } // mrdocs } // clang diff --git a/src/lib/Gen/hbs/MultiPageVisitor.hpp b/src/lib/Gen/hbs/MultiPageVisitor.hpp index 74fb7d07d7..60239a9f50 100644 --- a/src/lib/Gen/hbs/MultiPageVisitor.hpp +++ b/src/lib/Gen/hbs/MultiPageVisitor.hpp @@ -32,11 +32,6 @@ class MultiPageVisitor std::string_view outputPath_; Corpus const& corpus_; - void - writePage( - std::string_view text, - std::string_view filename); - public: MultiPageVisitor( ExecutorGroup& ex, @@ -54,16 +49,9 @@ class MultiPageVisitor respective tasks are also pushed to the executor group. */ - template T> + template + requires std::derived_from || std::same_as void operator()(T const& I); - - /** Push a task for the specified OverloadSet to the executor group. - - If the OverloadSet object refers to other Info objects, their - respective tasks are also pushed to the executor group. - - */ - void operator()(OverloadSet const& OS); }; } // hbs diff --git a/src/lib/Gen/hbs/SinglePageVisitor.cpp b/src/lib/Gen/hbs/SinglePageVisitor.cpp index 86974ed239..0899ba57ad 100644 --- a/src/lib/Gen/hbs/SinglePageVisitor.cpp +++ b/src/lib/Gen/hbs/SinglePageVisitor.cpp @@ -11,45 +11,63 @@ #include "SinglePageVisitor.hpp" #include +#include namespace clang { namespace mrdocs { namespace hbs { template +requires std::derived_from || std::same_as void SinglePageVisitor:: -operator()(T const& I) +operator()(T const& I0) { - ex_.async([this, &I, page = numPages_++](Builder& builder) + // If T is an OverloadSet, we make a copy for the lambda because + // these are temporary objects that don't live in the corpus. + // Otherwise, the lambda will capture a reference to the corpus Info. + auto Ref = [&I0] { + if constexpr (std::derived_from) + { + return std::ref(I0); + } + else if constexpr (std::same_as) + { + return OverloadSet(I0); + } + }(); + ex_.async([this, Ref, symbolIdx = numSymbols_++](Builder& builder) { - if(auto r = builder(I)) - writePage(*r, page); + T const& I = Ref; + + // Output to an independent string first, then write to + // the shared stream + std::stringstream ss; + if(auto r = builder(ss, I)) + { + writePage(ss.str(), symbolIdx); + } else + { r.error().Throw(); + } }); - if constexpr( + + if constexpr (std::derived_from) + { + if constexpr( T::isNamespace() || T::isRecord() || T::isEnum()) + { + corpus_.traverseOverloads(I0, *this); + } + } + else if constexpr (std::same_as) { - // corpus_.traverse(I, *this); - corpus_.traverseOverloads(I, *this); + corpus_.traverse(I0, *this); } -} -void -SinglePageVisitor:: -operator()(OverloadSet const& OS) -{ - ex_.async([this, OS, page = numPages_++](Builder& builder) - { - if(auto r = builder(OS)) - writePage(*r, page); - else - r.error().Throw(); - corpus_.traverse(OS, *this); - }); } // pageNumber is zero-based @@ -57,46 +75,58 @@ void SinglePageVisitor:: writePage( std::string pageText, - std::size_t pageNumber) + std::size_t symbolIdx) { std::unique_lock lock(mutex_); - if(pageNumber > topPage_) + if (symbolIdx > topSymbol_) { - // defer this page - if( pages_.size() <= pageNumber) - pages_.resize(pageNumber + 1); - pages_[pageNumber] = std::move(pageText); + // Defer this symbol + if( symbols_.size() <= symbolIdx) + symbols_.resize(symbolIdx + 1); + symbols_[symbolIdx] = std::move(pageText); return; } - // write contiguous pages + // Write contiguous pages for(;;) { + // Write the current symbol { unlock_guard unlock(mutex_); - os_.write(pageText.data(), pageText.size()); - ++pageNumber; + os_.write( + pageText.data(), + static_cast(pageText.size())); + ++symbolIdx; } - topPage_ = pageNumber; - if(pageNumber >= pages_.size()) + + topSymbol_ = symbolIdx; + if(symbolIdx >= symbols_.size()) + { + // No deferred symbols to write return; - if(! pages_[pageNumber]) + } + + if(! symbols_[symbolIdx]) + { + // The next symbol is not set yet return; - pageText = std::move(*pages_[pageNumber]); - // VFALCO this is in theory not needed but + } + + // Render the next deferred symbol + pageText = std::move(*symbols_[symbolIdx]); + // VFALCO this is in theory not needed, but // I am paranoid about the std::move of the // string not resulting in a deallocation. - pages_[pageNumber].reset(); + symbols_[symbolIdx].reset(); } } -#define DEFINE(T) template void \ - SinglePageVisitor::operator()(T const&) - -#define INFO(Type) DEFINE(Type##Info); +#define INFO(T) template void SinglePageVisitor::operator()(T##Info const&); #include +template void SinglePageVisitor::operator()(OverloadSet const&); + } // hbs } // mrdocs } // clang diff --git a/src/lib/Gen/hbs/SinglePageVisitor.hpp b/src/lib/Gen/hbs/SinglePageVisitor.hpp index e333df5e9e..b408f844b1 100644 --- a/src/lib/Gen/hbs/SinglePageVisitor.hpp +++ b/src/lib/Gen/hbs/SinglePageVisitor.hpp @@ -31,13 +31,12 @@ class SinglePageVisitor ExecutorGroup& ex_; Corpus const& corpus_; std::ostream& os_; - std::size_t numPages_ = 0; + std::size_t numSymbols_ = 0; std::mutex mutex_; - std::size_t topPage_ = 0; - std::vector> pages_; + std::size_t topSymbol_ = 0; + std::vector> symbols_; - void writePage(std::string pageText, std::size_t pageNumber); + void writePage(std::string pageText, std::size_t symbolIdx); public: SinglePageVisitor( ExecutorGroup& ex, @@ -49,11 +48,15 @@ class SinglePageVisitor { } - template - void operator()(T const& I); + /** Push a task for the specified Info to the executor group. - void operator()(OverloadSet const& OS); + If the Info object refers to other Info objects, their + respective tasks are also pushed to the executor group. + */ + template + requires std::derived_from || std::same_as + void operator()(T const& I); }; } // hbs From 85b275fa31d6aa26efce869e58730d64b98ba600 Mon Sep 17 00:00:00 2001 From: alandefreitas Date: Fri, 22 Nov 2024 17:26:27 -0300 Subject: [PATCH 2/2] perf(Builder): load template layouts at construction --- src/lib/Gen/hbs/Builder.cpp | 28 ++++++++++++++++++++++++++-- src/lib/Gen/hbs/Builder.hpp | 1 + 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/lib/Gen/hbs/Builder.cpp b/src/lib/Gen/hbs/Builder.cpp index d3fbdba576..b4a39a9655 100644 --- a/src/lib/Gen/hbs/Builder.cpp +++ b/src/lib/Gen/hbs/Builder.cpp @@ -143,6 +143,29 @@ Builder( helpers::registerAntoraHelpers(hbs_); helpers::registerLogicalHelpers(hbs_); helpers::registerContainerHelpers(hbs_); + + // load templates + exp = forEachFile(layoutDir(), false, + [&](std::string_view pathName) -> Expected + { + // Get template relative path + std::filesystem::path relPath = pathName; + relPath = relPath.lexically_relative(layoutDir()); + + // Skip non-handlebars files + MRDOCS_CHECK_OR(relPath.extension() == ".hbs", {}); + + // Load template contents + MRDOCS_TRY(std::string text, files::getFileText(pathName)); + + // Register template + this->templates_.emplace(relPath.generic_string(), text); + return {}; + }); + if (!exp) + { + exp.error().Throw(); + } } //------------------------------------------------ @@ -154,8 +177,9 @@ callTemplate( std::string_view name, dom::Value const& context) { - auto pathName = files::appendPath(layoutDir(), name); - MRDOCS_TRY(auto fileText, files::getFileText(pathName)); + auto it = templates_.find(name); + MRDOCS_CHECK(it != templates_.end(), formatError("Template {} not found", name)); + std::string_view fileText = it->second; HandlebarsOptions options; options.escapeFunction = escapeFn_; OutputRef out(os); diff --git a/src/lib/Gen/hbs/Builder.hpp b/src/lib/Gen/hbs/Builder.hpp index bb3f198135..b52de81888 100644 --- a/src/lib/Gen/hbs/Builder.hpp +++ b/src/lib/Gen/hbs/Builder.hpp @@ -33,6 +33,7 @@ class Builder { js::Context ctx_; Handlebars hbs_; + std::map> templates_; std::function escapeFn_; std::string