From 4de1ba776c60e935e8cf069c1aef104ee2b07559 Mon Sep 17 00:00:00 2001 From: Rye Date: Sun, 11 Jan 2026 13:38:37 -0500 Subject: Heavily reduce temporary allocations during LLSD parsing operations by utilizing moves and reducing temporary allocations --- indra/llcommon/llsd.cpp | 135 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 128 insertions(+), 7 deletions(-) (limited to 'indra/llcommon/llsd.cpp') diff --git a/indra/llcommon/llsd.cpp b/indra/llcommon/llsd.cpp index 77fe545c3f..6bc2841081 100644 --- a/indra/llcommon/llsd.cpp +++ b/indra/llcommon/llsd.cpp @@ -39,6 +39,9 @@ #include +#include +#include + // Defend against a caller forcibly passing a negative number into an unsigned // size_t index param inline @@ -103,6 +106,9 @@ protected: U32 mUseCount; public: + static void destruct(Impl*& var); + ///< safely decrement or destroy var + static void reset(Impl*& var, Impl* impl); ///< safely set var to refer to the new impl (possibly shared) @@ -166,7 +172,7 @@ public: virtual const LLSD& ref(size_t) const { return undef(); } virtual LLSD::map_const_iterator beginMap() const { return endMap(); } - virtual LLSD::map_const_iterator endMap() const { static const std::map empty; return empty.end(); } + virtual LLSD::map_const_iterator endMap() const { static const LLSD::llsd_map_t empty; return empty.end(); } virtual LLSD::array_const_iterator beginArray() const { return endArray(); } virtual LLSD::array_const_iterator endArray() const { static const std::vector empty; return empty.end(); } @@ -346,7 +352,7 @@ namespace LLSD::Real ImplString::asReal() const { F64 v = 0.0; - std::istringstream i_stream(mValue); + boost::iostreams::stream i_stream(mValue.data(), mValue.size()); i_stream >> v; // we would probably like to ignore all trailing whitespace as @@ -431,7 +437,7 @@ namespace class ImplMap final : public LLSD::Impl { private: - typedef std::map> DataMap; + using DataMap = LLSD::llsd_map_t; DataMap mData; @@ -457,7 +463,7 @@ namespace << it.second.asXMLRPCValue() << ""; } os << ""; - return os.str(); + return std::move(os).str(); } virtual bool has(std::string_view) const; @@ -467,8 +473,13 @@ namespace using LLSD::Impl::ref; // Unhiding ref(size_t) virtual LLSD get(std::string_view) const; virtual LLSD getKeys() const; + void insert(std::string&& k, const LLSD& v); + void insert(std::string&& k, LLSD&& v); void insert(std::string_view k, const LLSD& v); + void insert(std::string_view k, LLSD&& v); virtual void erase(const LLSD::String&); + LLSD& ref(std::string&&); + virtual const LLSD& ref(std::string&&) const; LLSD& ref(std::string_view); virtual const LLSD& ref(std::string_view) const; @@ -525,18 +536,58 @@ namespace return keys; } + void ImplMap::insert(std::string&& k, const LLSD& v) + { + LL_PROFILE_ZONE_SCOPED_CATEGORY_LLSD; + mData.emplace(std::move(k), v); + } + + void ImplMap::insert(std::string&& k, LLSD&& v) + { + LL_PROFILE_ZONE_SCOPED_CATEGORY_LLSD; + mData.emplace(std::move(k), std::move(v)); + } + void ImplMap::insert(std::string_view k, const LLSD& v) { LL_PROFILE_ZONE_SCOPED_CATEGORY_LLSD; mData.emplace(k, v); } + void ImplMap::insert(std::string_view k, LLSD&& v) + { + LL_PROFILE_ZONE_SCOPED_CATEGORY_LLSD; + mData.emplace(k, std::move(v)); + } + void ImplMap::erase(const LLSD::String& k) { LL_PROFILE_ZONE_SCOPED_CATEGORY_LLSD; mData.erase(k); } + LLSD& ImplMap::ref(std::string&& k) + { + DataMap::iterator i = mData.lower_bound(k); + if (i == mData.end() || mData.key_comp()(k, i->first)) + { + return mData.emplace_hint(i, std::make_pair(std::move(k), LLSD()))->second; + } + + return i->second; + } + + const LLSD& ImplMap::ref(std::string&& k) const + { + DataMap::const_iterator i = mData.lower_bound(k); + if (i == mData.end() || mData.key_comp()(k, i->first)) + { + return undef(); + } + + return i->second; + } + LLSD& ImplMap::ref(std::string_view k) { DataMap::iterator i = mData.lower_bound(k); @@ -598,7 +649,7 @@ namespace ImplArray(const DataVector& data) : mData(data) { } public: - ImplArray() { } + ImplArray() = default; virtual ImplArray& makeArray(Impl*&); @@ -615,7 +666,7 @@ namespace os << it.asXMLRPCValue(); } os << ""; - return os.str(); + return std::move(os).str(); } using LLSD::Impl::get; // Unhiding get(LLSD::String) @@ -625,10 +676,13 @@ namespace virtual LLSD get(size_t) const; void set(size_t, const LLSD&); void insert(size_t, const LLSD&); + void insert(size_t, LLSD&&); LLSD& append(const LLSD&); + LLSD& append(LLSD&&); virtual void erase(size_t); LLSD& ref(size_t); virtual const LLSD& ref(size_t) const; + void reserve(size_t size) { mData.reserve(size); } LLSD::array_iterator beginArray() { return mData.begin(); } LLSD::array_iterator endArray() { return mData.end(); } @@ -690,12 +744,31 @@ namespace mData.insert(mData.begin() + index, v); } + void ImplArray::insert(size_t i, LLSD&& v) + { + NEGATIVE_EXIT(i); + DataVector::size_type index = i; + + if (index >= mData.size()) // tbd - sanity check limit for index ? + { + mData.resize(index + 1); + } + + mData.insert(mData.begin() + index, std::move(v)); + } + LLSD& ImplArray::append(const LLSD& v) { mData.push_back(v); return mData.back(); } + LLSD& ImplArray::append(LLSD&& v) + { + mData.push_back(std::move(v)); + return mData.back(); + } + void ImplArray::erase(size_t i) { NEGATIVE_EXIT(i); @@ -763,6 +836,14 @@ LLSD::Impl::~Impl() --sOutstandingCount; } +void LLSD::Impl::destruct(Impl*& var) +{ + if (var && var->mUseCount != STATIC_USAGE_COUNT && --var->mUseCount == 0) + { + delete var; + } +} + void LLSD::Impl::reset(Impl*& var, Impl* impl) { if (impl && impl->mUseCount != STATIC_USAGE_COUNT) @@ -961,7 +1042,7 @@ namespace LLSD::LLSD() : impl(0) { ALLOC_LLSD_OBJECT; } -LLSD::~LLSD() { FREE_LLSD_OBJECT; Impl::reset(impl, 0); } +LLSD::~LLSD() { FREE_LLSD_OBJECT; Impl::destruct(impl); } LLSD::LLSD(const LLSD& other) : impl(0) { ALLOC_LLSD_OBJECT; assign(other); } void LLSD::assign(const LLSD& other) { Impl::assign(impl, other.impl); } @@ -1037,13 +1118,31 @@ LLSD LLSD::emptyMap() bool LLSD::has(const std::string_view k) const { return safe(impl).has(k); } LLSD LLSD::get(const std::string_view k) const { return safe(impl).get(k); } LLSD LLSD::getKeys() const { return safe(impl).getKeys(); } +void LLSD::insert(std::string&& k, const LLSD& v) { makeMap(impl).insert(std::move(k), v); } +void LLSD::insert(std::string&& k, LLSD&& v) { makeMap(impl).insert(std::move(k), std::move(v)); } void LLSD::insert(std::string_view k, const LLSD& v) { makeMap(impl).insert(k, v); } +void LLSD::insert(std::string_view k, LLSD&& v) { makeMap(impl).insert(k, std::move(v)); } +LLSD& LLSD::with(std::string&& k, const LLSD& v) + { + makeMap(impl).insert(std::move(k), v); + return *this; + } +LLSD& LLSD::with(std::string&& k, LLSD&& v) + { + makeMap(impl).insert(std::move(k), std::move(v)); + return *this; + } LLSD& LLSD::with(std::string_view k, const LLSD& v) { makeMap(impl).insert(k, v); return *this; } +LLSD& LLSD::with(std::string_view k, LLSD&& v) + { + makeMap(impl).insert(k, std::move(v)); + return *this; + } void LLSD::erase(const String& k) { makeMap(impl).erase(k); } LLSD& LLSD::operator[](const std::string_view k) @@ -1051,6 +1150,13 @@ LLSD& LLSD::operator[](const std::string_view k) LL_PROFILE_ZONE_SCOPED_CATEGORY_LLSD; return makeMap(impl).ref(k); } + +LLSD& LLSD::operator[](std::string&& k) +{ + LL_PROFILE_ZONE_SCOPED_CATEGORY_LLSD; + return makeMap(impl).ref(std::move(k)); +} + const LLSD& LLSD::operator[](const std::string_view k) const { LL_PROFILE_ZONE_SCOPED_CATEGORY_LLSD; @@ -1064,18 +1170,33 @@ LLSD LLSD::emptyArray() return v; } +LLSD LLSD::emptyReservedArray(size_t size) +{ + LLSD v; + makeArray(v.impl).reserve(size); + return v; +} + size_t LLSD::size() const { return safe(impl).size(); } LLSD LLSD::get(Integer i) const { return safe(impl).get(i); } void LLSD::set(Integer i, const LLSD& v){ makeArray(impl).set(i, v); } +void LLSD::set(Integer i, LLSD&& v) { makeArray(impl).set(i, std::move(v)); } void LLSD::insert(Integer i, const LLSD& v) { makeArray(impl).insert(i, v); } +void LLSD::insert(Integer i, LLSD&& v) { makeArray(impl).insert(i, std::move(v)); } LLSD& LLSD::with(Integer i, const LLSD& v) { makeArray(impl).insert(i, v); return *this; } +LLSD& LLSD::with(Integer i, LLSD&& v) + { + makeArray(impl).insert(i, std::move(v)); + return *this; + } LLSD& LLSD::append(const LLSD& v) { return makeArray(impl).append(v); } +LLSD& LLSD::append(LLSD&& v) { return makeArray(impl).append(std::move(v)); } void LLSD::erase(Integer i) { makeArray(impl).erase(i); } LLSD& LLSD::operator[](size_t i) -- cgit v1.3 From 0b2f7fcc6b67f629b41a804f77214d51497127a7 Mon Sep 17 00:00:00 2001 From: Rye Date: Mon, 12 Jan 2026 18:33:39 -0500 Subject: Address feedback from review Move LLSD string->real conversion function to shared impl and utilize in xml parsing Introducing additional error handling to menu init Cleanup comments and trace tagging Remove dead Memory menu entry --- indra/llcommon/llsd.cpp | 29 +++++---- indra/llcommon/llsd.h | 2 + indra/llcommon/llsdserialize_xml.cpp | 37 ++++-------- indra/newview/llinventorymodel.cpp | 2 +- indra/newview/llviewermenu.cpp | 69 ++++++++++++++++++++-- indra/newview/skins/default/xui/en/menu_viewer.xml | 12 ---- 6 files changed, 95 insertions(+), 56 deletions(-) (limited to 'indra/llcommon/llsd.cpp') diff --git a/indra/llcommon/llsd.cpp b/indra/llcommon/llsd.cpp index 6bc2841081..ce39bc7af3 100644 --- a/indra/llcommon/llsd.cpp +++ b/indra/llcommon/llsd.cpp @@ -351,18 +351,7 @@ namespace LLSD::Real ImplString::asReal() const { - F64 v = 0.0; - boost::iostreams::stream i_stream(mValue.data(), mValue.size()); - i_stream >> v; - - // we would probably like to ignore all trailing whitespace as - // well, but for now, simply eat the next character, and make - // sure we reached the end of the string. - // *NOTE: gcc 2.95 does not generate an eof() event on the - // stream operation above, so we manually get here to force it - // across platforms. - int c = i_stream.get(); - return ((EOF ==c) ? v : 0.0); + return llsd::string_to_real(mValue); } @@ -1264,6 +1253,22 @@ LLSD::reverse_array_iterator LLSD::rendArray() { return makeArray(impl) namespace llsd { +LLSD::Real string_to_real(std::string_view in_string) +{ + LLSD::Real v = 0.0; + boost::iostreams::stream i_stream(in_string.data(), in_string.size()); + i_stream >> v; + + // we would probably like to ignore all trailing whitespace as + // well, but for now, simply eat the next character, and make + // sure we reached the end of the string. + // *NOTE: gcc 2.95 does not generate an eof() event on the + // stream operation above, so we manually get here to force it + // across platforms. + int c = i_stream.get(); + return ((EOF == c) ? v : 0.0); +} + U32 allocationCount() { return LLSD::Impl::sAllocationCount; } U32 outstandingCount() { return LLSD::Impl::sOutstandingCount; } diff --git a/indra/llcommon/llsd.h b/indra/llcommon/llsd.h index c62f2ef1ca..afe35a65ba 100644 --- a/indra/llcommon/llsd.h +++ b/indra/llcommon/llsd.h @@ -545,6 +545,8 @@ LL_COMMON_API std::ostream& operator<<(std::ostream& s, const LLSD& llsd); namespace llsd { + // Used by LLSD::ImplString to convert string type to real + LLSD::Real string_to_real(std::string_view in_string); #ifdef LLSD_DEBUG_INFO /** @name Unit Testing Interface */ diff --git a/indra/llcommon/llsdserialize_xml.cpp b/indra/llcommon/llsdserialize_xml.cpp index c28efa3aad..f399c51608 100644 --- a/indra/llcommon/llsdserialize_xml.cpp +++ b/indra/llcommon/llsdserialize_xml.cpp @@ -712,7 +712,7 @@ void LLSDXMLParser::Impl::endElementHandler(const XML_Char* name) case ELEMENT_KEY: mCurrentKey = std::move(mCurrentContent); // This is safe to move as we are in the end element handler - mCurrentContent.clear(); // Clear to reset to valid state + mCurrentContent.clear(); // Ensure mCurrentContent is empty for subsequent use return; default: @@ -745,38 +745,21 @@ void LLSDXMLParser::Impl::endElementHandler(const XML_Char* name) } else { - // This implementation is copied from llsd.cpp - F64 v = 0.0; - boost::iostreams::stream i_stream(mCurrentContent.data(), mCurrentContent.size()); - i_stream >> v; - - // we would probably like to ignore all trailing whitespace as - // well, but for now, simply eat the next character, and make - // sure we reached the end of the string. - // *NOTE: gcc 2.95 does not generate an eof() event on the - // stream operation above, so we manually get here to force it - // across platforms. - int c = i_stream.get(); - value = (int)((EOF == c) ? v : 0.0); + // This must treat "1.23" not as an error, but as a number, which is + // then truncated down to an integer. Hence, this code doesn't call + // std::istringstream::operator>>(int&), which would not consume the + // ".23" portion. + + // Utilizes implementation used internally by LLSD::ImplString::asInteger + value = (int)llsd::string_to_real(mCurrentContent); } } break; case ELEMENT_REAL: { - // This implementation is copied from llsd.cpp - F64 v = 0.0; - boost::iostreams::stream i_stream(mCurrentContent.data(), mCurrentContent.size()); - i_stream >> v; - - // we would probably like to ignore all trailing whitespace as - // well, but for now, simply eat the next character, and make - // sure we reached the end of the string. - // *NOTE: gcc 2.95 does not generate an eof() event on the - // stream operation above, so we manually get here to force it - // across platforms. - int c = i_stream.get(); - value = ((EOF == c) ? v : 0.0); + // Utilizes implementation used internally by LLSD::ImplString::asReal + value = llsd::string_to_real(mCurrentContent); // removed since this breaks when locale has decimal separator that isn't '.' // investigated changing local to something compatible each time but deemed higher diff --git a/indra/newview/llinventorymodel.cpp b/indra/newview/llinventorymodel.cpp index e80c506bc0..c2f9c483c0 100644 --- a/indra/newview/llinventorymodel.cpp +++ b/indra/newview/llinventorymodel.cpp @@ -3456,7 +3456,7 @@ bool LLInventoryModel::loadFromFile(const std::string& filename, } { - LL_PROFILE_ZONE_NAMED("inventory load from file - categories"); + LL_PROFILE_ZONE_NAMED("inventory load from file - items"); const LLSD& llsd_items = inventory["items"]; if (llsd_items.isArray()) { diff --git a/indra/newview/llviewermenu.cpp b/indra/newview/llviewermenu.cpp index 1b496ccd27..5799e23ca5 100644 --- a/indra/newview/llviewermenu.cpp +++ b/indra/newview/llviewermenu.cpp @@ -413,7 +413,23 @@ static LLSLMMenuUpdater* gSLMMenuUpdater = NULL; LLSLMMenuUpdater::LLSLMMenuUpdater() { - mMarketplaceListingsItem = gMenuHolder->getChild("Me")->getChild("MarketplaceListings")->getHandle(); + LLView* me_menu = gMenuHolder->findChild("Me"); + if (!me_menu) + { + LLError::LLUserWarningMsg::showMissingFiles(); + LL_ERRS() << "Can't find 'Me' menu in 'menu_viewer'" << LL_ENDL; + return; + } + + LLView* marketplace_listings = me_menu->findChild("MarketplaceListings"); + if (!marketplace_listings) + { + LLError::LLUserWarningMsg::showMissingFiles(); + LL_ERRS() << "Can't find 'MarketplaceListings' in 'Me' menu" << LL_ENDL; + return; + } + + mMarketplaceListingsItem = marketplace_listings->getHandle(); } void LLSLMMenuUpdater::setMerchantMenu() { @@ -551,7 +567,29 @@ void init_menus() color = LLUIColorTable::instance().getColor( "MenuNonProductionBgColor" ); } - LLView* menu_bar_holder = gViewerWindow->getMainView()->findChildView("menu_stack")->findChildView("status_bar_container")->getChildView("menu_bar_holder"); + LLView* menu_stack = gViewerWindow->getMainView()->findChildView("menu_stack"); + if (!menu_stack) + { + LLError::LLUserWarningMsg::showMissingFiles(); + LL_ERRS() << "Can't find menu_stack in main_view" << LL_ENDL; + return; + } + + LLView* status_bar_container = menu_stack->findChildView("status_bar_container"); + if (!status_bar_container) + { + LLError::LLUserWarningMsg::showMissingFiles(); + LL_ERRS() << "Can't find status_bar_container in main_view" << LL_ENDL; + return; + } + + LLView* menu_bar_holder = status_bar_container->findChildView("menu_bar_holder"); + if (!menu_bar_holder) + { + LLError::LLUserWarningMsg::showMissingFiles(); + LL_ERRS() << "Can't find status_bar_container in main_view" << LL_ENDL; + return; + } gMenuBarView = LLUICtrlFactory::getInstance()->createFromFile("menu_viewer.xml", gMenuHolder, LLViewerMenuHolderGL::child_registry_t::instance()); gMenuBarView->setRect(LLRect(0, menu_bar_holder->getRect().mTop, 0, menu_bar_holder->getRect().mTop - MENU_BAR_HEIGHT)); @@ -567,8 +605,31 @@ void init_menus() const std::string animation_upload_cost_str = std::to_string(LLAgentBenefitsMgr::current().getAnimationUploadCost()); LLView* main_upload_menu = gMenuHolder->findChild("Upload"); - main_upload_menu->findChild("Upload Sound")->setLabelArg("[COST]", sound_upload_cost_str); - main_upload_menu->findChild("Upload Animation")->setLabelArg("[COST]", animation_upload_cost_str); + if (!main_upload_menu) + { + LLError::LLUserWarningMsg::showMissingFiles(); + LL_ERRS() << "Can't find 'Upload' menu in 'menu_viewer'" << LL_ENDL; + return; + } + + LLView* upload_sound = main_upload_menu->findChild("Upload Sound"); + if (!upload_sound) + { + LLError::LLUserWarningMsg::showMissingFiles(); + LL_ERRS() << "Can't find 'Upload Sound' menu item in 'Upload' menu" << LL_ENDL; + return; + } + upload_sound->setLabelArg("[COST]", sound_upload_cost_str); + + LLView* upload_anim = main_upload_menu->findChild("Upload Animation"); + if (!upload_anim) + { + LLError::LLUserWarningMsg::showMissingFiles(); + LL_ERRS() << "Can't find 'Upload Animation' menu item in 'Upload' menu" << LL_ENDL; + return; + } + upload_anim->setLabelArg("[COST]", animation_upload_cost_str); + gAttachSubMenu = gMenuBarView->findChildMenuByName("Attach Object", true); gDetachSubMenu = gMenuBarView->findChildMenuByName("Detach Object", true); diff --git a/indra/newview/skins/default/xui/en/menu_viewer.xml b/indra/newview/skins/default/xui/en/menu_viewer.xml index c93cb7822e..ba43e80eda 100644 --- a/indra/newview/skins/default/xui/en/menu_viewer.xml +++ b/indra/newview/skins/default/xui/en/menu_viewer.xml @@ -2600,18 +2600,6 @@ function="World.EnvPreset" function="Advanced.ToggleConsole" parameter="fast timers" /> - - - - -- cgit v1.3