Adding performance marks for clipboard operations #282872
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds performance marks to track clipboard operations (cut, copy, and paste) across different editor input handling mechanisms in VS Code. The marks are added to measure when these operations occur and include contextual details like operation IDs and text lengths.
Key Changes:
- Added performance marks to track clipboard operations at multiple architectural layers
- Included detail objects with operation metadata (id, length, command type) for better observability
- Added marks to both high-level controllers and low-level input handlers
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/editor/contrib/dropOrPasteInto/browser/copyPasteController.ts | Added performance marks for handleCopy and handlePaste methods with metadata details |
| src/vs/editor/contrib/clipboard/browser/clipboard.ts | Added marks for cut/copy/paste operations in registerExecCommandImpl and executeClipboardCopyWithWorkaround |
| src/vs/editor/browser/controller/editContext/textArea/textAreaEditContextInput.ts | Added marks for cut, copy, and paste event handlers in TextAreaInput |
| src/vs/editor/browser/controller/editContext/native/nativeEditContext.ts | Added marks for copy, cut, and paste event handlers in NativeEditContext |
| src/vs/editor/browser/controller/editContext/clipboardUtils.ts | Added mark for ensureClipboardGetsEditorSelection utility function |
| performance.mark('code/CopyPasteController/handleCopy', { detail: { id, length: text.length } }); | ||
| } else { | ||
| this._logService.trace('CopyPasteController#handleCopy'); | ||
| performance.mark('code/CopyPasteController/handleCopy'); |
There was a problem hiding this comment.
The performance mark placement may not accurately measure the intended operation. The mark is placed at the beginning of the function, before checking if the editor has text focus and before the actual clipboard operation. This means it will be marked even when the function returns early without performing any clipboard operation. Consider placing the mark after the early return checks (after line 187) to ensure it only measures actual clipboard copy operations.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| performance.mark('code/CopyPasteController/handlePaste', { detail: { id: metadataComputed?.id } }); | ||
| } else { | ||
| this._logService.trace('CopyPasteController#handlePaste'); | ||
| performance.mark('code/CopyPasteController/handlePaste'); |
There was a problem hiding this comment.
The performance mark placement may not accurately measure the intended operation. The mark is placed at the beginning of the function, before checking if clipboardData exists and if the editor has text focus. This means it will be marked even when the function returns early without performing any paste operation. Consider placing the mark after the early return checks (after line 272) to ensure it only measures actual paste operations.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
No description provided.