feat: Add Profiler Tool for Unity performance analysis via MCP #338
Conversation
|
@Ficksik it looks promising!
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive Profiler tool for Unity MCP that enables AI assistants to perform performance analysis through 12 new MCP tools. The implementation follows the project's established patterns with partial classes, thread-safe Unity API calls, and structured error handling. However, there are several moderate issues related to API consistency, misleading functionality, and missing test coverage that should be addressed.
Key Changes
- Added 12 new MCP profiler tools for performance monitoring (Start, Stop, GetStatus, memory/rendering/script statistics, frame capture, data persistence, and module management)
- Implemented local tracking of profiler state and enabled modules due to Unity API limitations
- Structured response types with proper JSON serialization for AI consumption
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Profiler.cs | Main class defining data models, error messages, and module configuration |
| Profiler.Start.cs | Enables Unity Profiler and opens the Profiler window |
| Profiler.Stop.cs | Disables Unity Profiler |
| Profiler.GetStatus.cs | Returns profiler status with active modules and memory usage |
| Profiler.GetMemoryStats.cs | Retrieves detailed memory statistics from Unity Profiler |
| Profiler.GetRenderingStats.cs | Returns rendering statistics including FPS and graphics info |
| Profiler.GetScriptStats.cs | Provides script execution statistics and memory usage |
| Profiler.CaptureFrame.cs | Captures current frame data snapshot |
| Profiler.SaveData.cs | Saves profiler statistics to JSON file |
| Profiler.LoadData.cs | Loads profiler data from JSON file |
| Profiler.ClearData.cs | Placeholder for clearing profiler data |
| Profiler.EnableModule.cs | Enables/disables profiler modules with local tracking |
| Profiler.ListModules.cs | Lists available modules with enabled status |
| *.meta files | Unity metadata files for all new scripts |
| public ResponseCallValueTool<MemoryStatsData?> GetMemoryStats() | ||
| { | ||
| return MainThread.Instance.Run(() => | ||
| { |
There was a problem hiding this comment.
The GetMemoryStats() method doesn't check if the profiler is enabled before accessing profiler data, unlike other stat methods (GetRenderingStats and GetScriptStats). While Unity's Profiler API methods like GetTotalReservedMemoryLong() can be called without enabling the profiler, this inconsistency may be confusing for API consumers. Consider adding the same profiler enabled check for consistency, or document why memory stats don't require it.
| { | |
| { | |
| if (!Profiler.enabled) | |
| { | |
| return ResponseCallValueTool<MemoryStatsData?>.Error("Unity Profiler is not enabled. Enable the Profiler to retrieve memory statistics."); | |
| } |
| ( | ||
| [Description("The number of frames to capture. Note: Currently only captures current frame data.")] | ||
| int frameCount = 1 | ||
| ) |
There was a problem hiding this comment.
The frameCount parameter in CaptureFrame is accepted but not used - the method always captures only the current frame. This creates a misleading API where users might expect to capture multiple frames but will always get a single frame snapshot. Either implement multi-frame capture functionality, remove the parameter entirely, or add validation that returns an error if frameCount != 1.
| "Profiler_ClearData", | ||
| Title = "Clear Profiler Data" | ||
| )] | ||
| [Description(@"Clears the profiler data. | ||
| Note: To clear profiler history, use the Clear button in Unity's Profiler window.")] | ||
| public string ClearData() | ||
| => MainThread.Instance.Run(() => | ||
| { | ||
| return "[Success] Profiler data cleared successfully.\nNote: To clear profiler history, use the Clear button in Unity's Profiler window."; |
There was a problem hiding this comment.
The ClearData() method doesn't actually clear any profiler data - it only returns a success message. This is misleading because the method name and description imply that data will be cleared, but no action is performed. Either implement actual data clearing functionality (e.g., clearing the local profilerEnabled state or calling Unity profiler APIs), or rename the method to indicate it's informational only.
| "Profiler_ClearData", | |
| Title = "Clear Profiler Data" | |
| )] | |
| [Description(@"Clears the profiler data. | |
| Note: To clear profiler history, use the Clear button in Unity's Profiler window.")] | |
| public string ClearData() | |
| => MainThread.Instance.Run(() => | |
| { | |
| return "[Success] Profiler data cleared successfully.\nNote: To clear profiler history, use the Clear button in Unity's Profiler window."; | |
| "Profiler_ShowClearDataInfo", | |
| Title = "Show Profiler Clear Data Info" | |
| )] | |
| [Description(@"Provides information on how to clear profiler data in Unity. | |
| Note: Profiler data cannot be cleared programmatically. Use the Clear button in Unity's Profiler window to clear profiler history.")] | |
| public string ShowClearDataInfo() | |
| => MainThread.Instance.Run(() => | |
| { | |
| return "[Info] Profiler data cannot be cleared programmatically.\nTo clear profiler history, use the Clear button in Unity's Profiler window."; |
| [McpPluginToolType] | ||
| public partial class Tool_Profiler | ||
| { | ||
| /// <summary> | ||
| /// Tracks profiler enabled state locally. | ||
| /// </summary> | ||
| private static bool profilerEnabled = false; | ||
|
|
||
| /// <summary> | ||
| /// Set of enabled profiler modules. | ||
| /// </summary> | ||
| private static readonly HashSet<string> enabledModules = new HashSet<string>() | ||
| { | ||
| "CPU", | ||
| "GPU", | ||
| "Rendering", | ||
| "Memory", | ||
| "Audio", | ||
| "Video", | ||
| "Physics", | ||
| "Physics2D", | ||
| "UI" | ||
| }; | ||
|
|
||
| /// <summary> | ||
| /// List of all available profiler modules. | ||
| /// </summary> | ||
| public static readonly List<string> AvailableModules = new List<string>() | ||
| { | ||
| "CPU", | ||
| "GPU", | ||
| "Rendering", | ||
| "Memory", | ||
| "Audio", | ||
| "Video", | ||
| "Physics", | ||
| "Physics2D", | ||
| "NetworkMessages", | ||
| "NetworkOperations", | ||
| "UI", | ||
| "UIDetails", | ||
| "GlobalIllumination", | ||
| "VirtualTexturing" | ||
| }; | ||
|
|
||
| public static class Error | ||
| { | ||
| public static string ProfilerNotEnabled() | ||
| => "[Error] Profiler must be enabled to perform this operation. Use 'Profiler_Start' first."; | ||
|
|
||
| public static string ModuleNameIsRequired() | ||
| => "[Error] Module name is required."; | ||
|
|
||
| public static string UnknownModule(string moduleName) | ||
| => $"[Error] Unknown profiler module: '{moduleName}'. Available modules: {string.Join(", ", AvailableModules)}"; | ||
|
|
||
| public static string FilePathIsRequired() | ||
| => "[Error] File path is required."; | ||
|
|
||
| public static string FileNotFound(string filePath) | ||
| => $"[Error] Profiler data file not found: '{filePath}'."; | ||
|
|
||
| public static string FailedToSaveData(string message) | ||
| => $"[Error] Failed to save profiler data: {message}"; | ||
|
|
||
| public static string FailedToLoadData(string message) | ||
| => $"[Error] Failed to load profiler data: {message}"; | ||
| } | ||
|
|
||
| [Description("Profiler status data including memory and module information.")] | ||
| public class ProfilerStatusData | ||
| { | ||
| [Description("Whether the profiler is enabled.")] | ||
| public bool Enabled { get; set; } | ||
|
|
||
| [Description("Whether Unity's runtime profiler is enabled.")] | ||
| public bool RuntimeProfilerEnabled { get; set; } | ||
|
|
||
| [Description("List of active profiler modules.")] | ||
| public List<string>? ActiveModules { get; set; } | ||
|
|
||
| [Description("Maximum used memory in MB.")] | ||
| public float MaxUsedMemoryMB { get; set; } | ||
|
|
||
| [Description("Whether profiling is supported on this platform.")] | ||
| public bool Supported { get; set; } | ||
| } | ||
|
|
||
| [Description("Memory statistics from the Unity Profiler.")] | ||
| public class MemoryStatsData | ||
| { | ||
| [Description("Total reserved memory in MB.")] | ||
| public float TotalReservedMemoryMB { get; set; } | ||
|
|
||
| [Description("Total allocated memory in MB.")] | ||
| public float TotalAllocatedMemoryMB { get; set; } | ||
|
|
||
| [Description("Total unused reserved memory in MB.")] | ||
| public float TotalUnusedReservedMemoryMB { get; set; } | ||
|
|
||
| [Description("Mono heap size in MB.")] | ||
| public float MonoHeapSizeMB { get; set; } | ||
|
|
||
| [Description("Mono used size in MB.")] | ||
| public float MonoUsedSizeMB { get; set; } | ||
|
|
||
| [Description("Temp allocator size in MB.")] | ||
| public float TempAllocatorSizeMB { get; set; } | ||
|
|
||
| [Description("Graphics memory for driver in MB.")] | ||
| public float GraphicsMemoryMB { get; set; } | ||
|
|
||
| [Description("Maximum used memory in MB.")] | ||
| public float MaxUsedMemoryMB { get; set; } | ||
|
|
||
| [Description("Used heap size in MB.")] | ||
| public float UsedHeapSizeMB { get; set; } | ||
| } | ||
|
|
||
| [Description("Rendering statistics from the Unity Profiler.")] | ||
| public class RenderingStatsData | ||
| { | ||
| [Description("Frame time in milliseconds.")] | ||
| public float FrameTimeMs { get; set; } | ||
|
|
||
| [Description("Frames per second.")] | ||
| public float Fps { get; set; } | ||
|
|
||
| [Description("VSync count setting.")] | ||
| public int VSyncCount { get; set; } | ||
|
|
||
| [Description("Target frame rate.")] | ||
| public int TargetFrameRate { get; set; } | ||
|
|
||
| [Description("Rendering threading mode.")] | ||
| public string? RenderingThreadingMode { get; set; } | ||
|
|
||
| [Description("Graphics device type.")] | ||
| public string? GraphicsDeviceType { get; set; } | ||
| } | ||
|
|
||
| [Description("Script statistics from the Unity Profiler.")] | ||
| public class ScriptStatsData | ||
| { | ||
| [Description("Frame time in milliseconds.")] | ||
| public float FrameTimeMs { get; set; } | ||
|
|
||
| [Description("Fixed delta time in milliseconds.")] | ||
| public float FixedDeltaTimeMs { get; set; } | ||
|
|
||
| [Description("Time scale.")] | ||
| public float TimeScale { get; set; } | ||
|
|
||
| [Description("Current frame count.")] | ||
| public int FrameCount { get; set; } | ||
|
|
||
| [Description("Real time since startup in seconds.")] | ||
| public float RealtimeSinceStartup { get; set; } | ||
|
|
||
| [Description("Mono memory usage in MB.")] | ||
| public float MonoMemoryUsageMB { get; set; } | ||
|
|
||
| [Description("GC memory usage in MB.")] | ||
| public float GCMemoryUsageMB { get; set; } | ||
| } | ||
|
|
||
| [Description("Frame capture data.")] | ||
| public class FrameCaptureData | ||
| { | ||
| [Description("Frame time in milliseconds.")] | ||
| public float FrameTimeMs { get; set; } | ||
|
|
||
| [Description("Frames per second.")] | ||
| public float Fps { get; set; } | ||
|
|
||
| [Description("Current frame count.")] | ||
| public int FrameCount { get; set; } | ||
|
|
||
| [Description("Real time since startup in seconds.")] | ||
| public float RealtimeSinceStartup { get; set; } | ||
|
|
||
| [Description("Rendered frame count.")] | ||
| public int RenderedFrameCount { get; set; } | ||
| } | ||
|
|
||
| [Description("Profiler module information.")] | ||
| public class ProfilerModuleInfo | ||
| { | ||
| [Description("Module name.")] | ||
| public string? Name { get; set; } | ||
|
|
||
| [Description("Whether the module is enabled.")] | ||
| public bool Enabled { get; set; } | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
The new Profiler tool lacks test coverage. The repository has comprehensive test coverage for other tools (GameObject, Console, Assets, etc.) in Assets/root/Tests/Editor/Tool/. Consider adding tests for the Profiler tool to ensure reliability and maintain consistency with the project's testing practices. Key scenarios to test include: profiler start/stop state management, error handling when profiler is not enabled, module enable/disable tracking, and data serialization.
| if (profilerEnabled) | ||
| return "[Success] Profiler is already running."; | ||
|
|
||
| profilerEnabled = true; |
There was a problem hiding this comment.
There's a potential race condition between the local profilerEnabled flag and Unity's Profiler.enabled state. If Unity's Profiler is enabled externally (e.g., through the Unity Editor UI), the local profilerEnabled flag will be out of sync, causing methods like GetScriptStats() to incorrectly return "Profiler must be enabled" errors even though Unity's profiler is actually running. Consider synchronizing with Profiler.enabled state in methods that check profilerEnabled, or remove the local flag and always check Profiler.enabled directly.
| if (profilerEnabled) | |
| return "[Success] Profiler is already running."; | |
| profilerEnabled = true; | |
| if (Profiler.enabled) | |
| return "[Success] Profiler is already running."; |
| [Description("Current frame count.")] | ||
| public int FrameCount { get; set; } |
There was a problem hiding this comment.
[nitpick] The property name FrameCount is ambiguous in the FrameCaptureData class. This field is set from Time.frameCount (total frames since start), but when paired with RenderedFrameCount, it's unclear what the distinction is. Consider renaming to TotalFrameCount or FramesSinceStart to clarify the difference from RenderedFrameCount.
| [Description("Current frame count.")] | |
| public int FrameCount { get; set; } | |
| [Description("Total frame count since start.")] | |
| public int TotalFrameCount { get; set; } |
|
@IvanMurzak I added tests |
| protected void StructuredResponseValidation<T>(ResponseCallValueTool<T?> response) | ||
| { | ||
| Debug.Log($"[{GetType().GetTypeShortName()}] Structured Response Status: {response.Status}"); | ||
| Assert.AreEqual(ResponseStatus.Success, response.Status, $"Response should be successful."); | ||
| Assert.IsNotNull(response.StructuredContent, "Response should have structured content."); | ||
| } | ||
|
|
||
| protected void StructuredResponseErrorValidation<T>(ResponseCallValueTool<T?> response) | ||
| { | ||
| Debug.Log($"[{GetType().GetTypeShortName()}] Structured Error Response Status: {response.Status}"); | ||
| Assert.AreEqual(ResponseStatus.Error, response.Status, $"Response should be error."); | ||
| } |
There was a problem hiding this comment.
The StructuredResponseValidation and StructuredResponseErrorValidation methods validate the response status but don't verify that the content is non-null for success cases or that the error message is meaningful for error cases.
Consider adding:
- For success validation:
Assert.IsNotNull(response.StructuredContent, "Structured content should not be null for successful responses."); - For error validation:
Assert.IsNotNull(response.Message, "Error message should not be null."); Assert.IsNotEmpty(response.Message, "Error message should not be empty.");
| /// <summary> | ||
| /// Tracks profiler enabled state locally. | ||
| /// </summary> | ||
| private static bool profilerEnabled = false; |
There was a problem hiding this comment.
The profilerEnabled static field is initialized to false, but there's no synchronization with Unity's actual Profiler.enabled state when the plugin initializes. If Unity's Profiler is already enabled when the MCP plugin starts (e.g., manually enabled via the UI), the local profilerEnabled flag will be out of sync.
Consider initializing profilerEnabled based on the actual Unity Profiler state, or checking Profiler.enabled in the status checks instead of relying solely on the local flag. For example, in Start() you could check: if (profilerEnabled || Profiler.enabled) to handle cases where the profiler was enabled outside of MCP.
| public ResponseCallValueTool<MemoryStatsData?> GetMemoryStats() | ||
| { | ||
| return MainThread.Instance.Run(() => | ||
| { | ||
| var data = new MemoryStatsData | ||
| { | ||
| TotalReservedMemoryMB = Profiler.GetTotalReservedMemoryLong() / 1048576f, | ||
| TotalAllocatedMemoryMB = Profiler.GetTotalAllocatedMemoryLong() / 1048576f, | ||
| TotalUnusedReservedMemoryMB = Profiler.GetTotalUnusedReservedMemoryLong() / 1048576f, | ||
| MonoHeapSizeMB = Profiler.GetMonoHeapSizeLong() / 1048576f, | ||
| MonoUsedSizeMB = Profiler.GetMonoUsedSizeLong() / 1048576f, | ||
| TempAllocatorSizeMB = Profiler.GetTempAllocatorSize() / 1048576f, | ||
| GraphicsMemoryMB = Profiler.GetAllocatedMemoryForGraphicsDriver() / 1048576f, | ||
| MaxUsedMemoryMB = Profiler.maxUsedMemory / 1048576f, | ||
| UsedHeapSizeMB = Profiler.usedHeapSizeLong / 1048576f | ||
| }; | ||
|
|
||
| var mcpPlugin = UnityMcpPlugin.Instance.McpPluginInstance | ||
| ?? throw new InvalidOperationException("MCP Plugin instance is not available."); | ||
| var jsonNode = mcpPlugin.McpManager.Reflector.JsonSerializer.SerializeToNode(data); | ||
| var jsonString = jsonNode?.ToJsonString(); | ||
| return ResponseCallValueTool<MemoryStatsData?>.SuccessStructured(jsonNode, jsonString); | ||
| }); |
There was a problem hiding this comment.
The GetMemoryStats() method doesn't check if the profiler is enabled before collecting statistics, unlike GetRenderingStats() and GetScriptStats() which both return an error when !profilerEnabled.
This inconsistency is confusing for API consumers. Memory statistics from Unity's Profiler API are available regardless of whether profiling is enabled, so either:
- Document this difference clearly in the method's Description attribute
- Make the behavior consistent by removing the profiler-enabled check from the other stats methods
- Add the check to GetMemoryStats for consistency (though this might be less useful since memory stats are always available)
| [SetUp] | ||
| public void SaveLoadSetUp() | ||
| { | ||
| _testFilePath = Path.Combine(Application.temporaryCachePath, "profiler_test_data.json"); | ||
| } | ||
|
|
||
| [TearDown] | ||
| public void SaveLoadTearDown() | ||
| { | ||
| // Clean up test file | ||
| if (File.Exists(_testFilePath)) | ||
| File.Delete(_testFilePath); | ||
| } |
There was a problem hiding this comment.
The TestToolProfiler class defines two [SetUp] methods: TestSetUp() at line 28 and SaveLoadSetUp() at line 25. Similarly, there are two [TearDown] methods: TestTearDown() and SaveLoadTearDown().
NUnit will execute both SetUp methods before each test and both TearDown methods after each test, which means _testFilePath will be initialized for ALL tests, not just the SaveLoadData tests. This works but is inefficient.
Consider renaming SaveLoadSetUp() and SaveLoadTearDown() to not use the [SetUp]/[TearDown] attributes, and instead call them explicitly from the tests that need them, or make them private helper methods called from the individual test methods that need the file path setup.
| protected void ResultValidation(string? result) | ||
| { | ||
| Debug.Log($"[{GetType().GetTypeShortName()}] Result:\n{result}"); | ||
| Assert.IsNotNull(result, "Result should not be null."); | ||
| Assert.IsNotEmpty(result, "Result should not be empty."); | ||
| Assert.IsTrue(result!.Contains("[Success]"), $"Should contain success message.\nResult: {result}"); | ||
| Assert.IsFalse(result.Contains("[Error]"), $"Should not contain error message.\nResult: {result}"); | ||
| } | ||
|
|
||
| protected void ResultValidationExpected(string? result, params string[] expectedLines) | ||
| { | ||
| Debug.Log($"[{GetType().GetTypeShortName()}] Result:\n{result}"); | ||
| Assert.IsNotNull(result, "Result should not be null."); | ||
| Assert.IsNotEmpty(result, "Result should not be empty."); | ||
| Assert.IsTrue(result!.Contains("[Success]"), $"Should contain success message.\nResult: {result}"); | ||
|
|
||
| foreach (var line in expectedLines) | ||
| Assert.IsTrue(result.Contains(line), $"Should contain expected line: {line}\nResult: {result}"); | ||
| } | ||
|
|
||
| protected void ErrorValidation(string? result, string expectedErrorPart = "[Error]") | ||
| { | ||
| Debug.Log($"[{GetType().GetTypeShortName()}] Error Result:\n{result}"); | ||
| Assert.IsNotNull(result, "Result should not be null."); | ||
| Assert.IsTrue(result!.Contains(expectedErrorPart), $"Should contain error part: {expectedErrorPart}\nResult: {result}"); |
There was a problem hiding this comment.
[nitpick] The ResultValidation method on line 45 checks result!.Contains("[Success]") but the previous line already asserts that result is not null. The null-forgiving operator ! is unnecessary here since the assertion guarantees non-null.
Similarly on lines 54 and 64, the null-forgiving operator is used after null checks. While this doesn't cause runtime issues, it's redundant. Consider removing the ! operators after assertions that guarantee non-null values.
| foreach (var moduleName in Tool_Profiler.AvailableModules) | ||
| { | ||
| // Act | ||
| var result = _tool.EnableModule(moduleName, enabled: true); | ||
|
|
||
| // Assert | ||
| ResultValidation(result); | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| contentToDeserialize = resultProp.GetRawText(); | ||
| } | ||
| } | ||
| catch { } |
There was a problem hiding this comment.
Poor error handling: empty catch block.
2 participants
Summary
This PR introduces a new
Tool_ProfilerMCP tool that provides comprehensive Unity Profiler management capabilities through the MCP interface, enabling AI assistants to help with performance analysis and debugging.Features
New MCP Tools (12 tools)
Profiler_StartProfiler_StopProfiler_GetStatusProfiler_GetMemoryStatsProfiler_GetRenderingStatsProfiler_GetScriptStatsProfiler_CaptureFrameProfiler_SaveDataProfiler_LoadDataProfiler_ClearDataProfiler_EnableModuleProfiler_ListModulesSupported Profiler Modules
CPU, GPU, Rendering, Memory, Audio, Video, Physics, Physics2D, NetworkMessages, NetworkOperations, UI, UIDetails, GlobalIllumination, VirtualTexturing
Implementation Details
MainThread.Instance.Run()for thread safety[Description]attributes for AI guidanceErrorclassFiles Added
Profiler.cs- Main class with data models and error definitionsProfiler.Start.csProfiler.Stop.csProfiler.GetStatus.csProfiler.GetMemoryStats.csProfiler.GetRenderingStats.csProfiler.GetScriptStats.csProfiler.CaptureFrame.csProfiler.SaveData.csProfiler.LoadData.csProfiler.ClearData.csProfiler.EnableModule.csProfiler.ListModules.csNotes
Testing