-
Notifications
You must be signed in to change notification settings - Fork 99
Performance optimization pass and cleanup for #5151 #5194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Performance optimization pass and cleanup for #5151 #5194
Conversation
…ocations and utf8->utf16 conversions Fix LLFile::tmpdir failing when temp directory is located in a unicode path on windows Add Tracy profiler tagging to LLFile that's disabled by default Replace LLFile::utf8StringToWstring with our fsyspath std::filesystem::path adapter Introduce std::filesystem::path to llifstream/llofstream wrappers Add move operators to fsyspath to match copy operator Restore LLUniqueFile RAII class for direct c file io wrapping and reducing merge conflicts with downstream code that may depend upon it Signed-off-by: Rye <rye@alchemyviewer.org>
…he with new LLFile IO code Add non-allocating to_chars and to_wchars string creation functions to LLUUID Signed-off-by: Rye <rye@alchemyviewer.org>
Signed-off-by: Rye <rye@alchemyviewer.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR performs a comprehensive performance optimization pass related to issue #5151, focusing on replacing boost::filesystem with std::filesystem throughout the codebase, reducing temporary allocations and UTF8↔UTF16 conversions, and improving file I/O operations.
Key changes:
- Complete migration from boost::filesystem to std::filesystem across all platforms
- Introduction of TEXT() macro and std::filesystem::path parameter support to LLFile APIs
- Addition of non-allocating LLUUID::to_chars() and to_wchars() methods to reduce string allocations during path construction
- Fix for LLFile::tmpdir() failing with Unicode paths on Windows
- Introduction of LLUniqueFile RAII wrapper for FILE* safety
Reviewed changes
Copilot reviewed 70 out of 70 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| indra/llcommon/llfile.h | Major API refactor: Added std::filesystem::path overloads, TEXT macro for platform-specific mode strings, removed LL_COMMON_API attributes |
| indra/llcommon/llfile.cpp | Implementation of new path-based APIs, fix for tmpdir() Unicode support on Windows, Tracy profiler integration |
| indra/llcommon/lluuid.h/cpp | Added to_chars() and to_wchars() for non-allocating UUID string generation |
| indra/llcommon/fsyspath.h | Added move operators to match copy operator |
| indra/llfilesystem/* | Complete migration from boost::filesystem to std::filesystem |
| indra/newview/lltexturecache.* | Optimized to use std::filesystem::path and new LLUUID methods to reduce allocations |
| indra/test/namedtempfile.h | Migrated from boost to std filesystem, changed getName() return type to const std::filesystem::path& |
| Multiple fopen call sites | Updated to use TEXT() macro for proper wide-char mode strings on Windows |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Don't forget raw-string syntax for Windows pathnames. | ||
| // N.B. Using 'print' implicitly adds newlines. | ||
| "with open(r'" << file.getName() << "', 'wb') as f:\n" | ||
| "with open(r'" << (const char*)file.getPath().u8string().c_str() << "', 'wb') as f:\n" |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Casting to const char* from u8string() is unsafe and may cause issues. The u8string() method returns a std::u8string, and c_str() returns const char8_t*, not const char*. This explicit cast can lead to undefined behavior or incorrect string handling. Consider using the string() method instead or properly handling the u8string type.
| "with open(r'" << (const char*)file.getPath().u8string().c_str() << "', 'wb') as f:\n" | |
| "with open(r'" << file.getPath().string().c_str() << "', 'wb') as f:\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The (const char*)u8string()._c_str() is to engage std::filesystems utf8 conversion machinery...unfortunately c++ still hasn't made that easy to work with.
|
Signed-off-by: Rye <rye@alchemyviewer.org>
8948232 to
b621d13
Compare
|
Fixed! |
Description
Related Issues
Issue Link: #5151
Checklist
Please ensure the following before requesting review:
Additional Notes