Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves profiler performance by migrating trace processing from JavaScript to C++, reducing string copying and improving memory stability for long-running traces.
Changes:
- Refactored the tracing agent to store traces in C++ instead of processing them in JavaScript
- Updated inspector interfaces to use const references for better performance
- Added JSON library (nlohmann/json) to third_party for JSON parsing in C++
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| v8ios.xcodeproj/project.pbxproj | Added third_party directory to project structure |
| NativeScript/inspector/ns-v8-tracing-agent-impl.mm | Refactored trace writer to use chunked storage and removed JavaScript trace processing |
| NativeScript/inspector/ns-v8-tracing-agent-impl.h | Updated interfaces to return vector of strings and added chunking methods |
| NativeScript/inspector/JsV8InspectorClient.mm | Added JSON parsing for trace configuration and removed JavaScript-based trace processing |
| NativeScript/inspector/JsV8InspectorClient.h | Updated method signatures to use const references |
| NativeScript/inspector/InspectorServer.mm | Updated method signatures to use const references |
| NativeScript/inspector/InspectorServer.h | Updated method signatures to use const references |
| .clang-format-ignore | Added third_party directory to format ignore list |
Comments suppressed due to low confidence (1)
NativeScript/inspector/JsV8InspectorClient.mm:1
- Lambda captures
messageby value, creating an unnecessary string copy. SinceSendtakes aconst std::string&, the lambda parameter should also beconst std::string&:[channel, q](const std::string& message).
#include <Foundation/Foundation.h>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
18f69c1 to
b1504bb
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This improves the profiler by reducing string copying as well as running it all in C++ (no more entering JS to generate the output). As a result, memory is much more stable and performance is improved, allowing you to run long and accurate traces