Fix code lens perf for interactive window#11142
Conversation
| TextEditorRevealType | ||
| } from 'vscode'; | ||
|
|
||
| import { IDisposable } from 'monaco-editor'; |
There was a problem hiding this comment.
IDisposable [](start = 9, length = 11)
Crap, Wrong one.
| return Uri.parse(Identifiers.InteractiveWindowIdentity); | ||
| return { | ||
| resource: Uri.parse(Identifiers.InteractiveWindowIdentity), | ||
| type: 'interactive' |
There was a problem hiding this comment.
We need a type here so that the codeLensFactory can only initiailize it's hash provider when the notebook for the interactive window is created. It's a singleton so we don't want to reinit the hash provider on every notebook open.
| .filter((e) => e.hashes.length > 0); | ||
| } | ||
|
|
||
| public restartedKernel() { |
There was a problem hiding this comment.
| public restartedKernel() { | |
| public onKernelRestarted() { |
As we're reacting to an event that took place (i.e. this is meant to be an event handler), I think onKernelRestarted is better
There was a problem hiding this comment.
Codecov Report
@@ Coverage Diff @@
## master #11142 +/- ##
===========================================
- Coverage 61.47% 0.00% -61.48%
===========================================
Files 597 11 -586
Lines 32801 37 -32764
Branches 4640 4 -4636
===========================================
- Hits 20163 0 -20163
+ Misses 11622 37 -11585
+ Partials 1016 0 -1016
Continue to review full report at Codecov.
|
| } | ||
| private onChangedSettings() { | ||
| // When config settings change, refresh our code lenses. | ||
| this.codeLensCache.clear(); |
There was a problem hiding this comment.
This looks like it's on any settings change.
There was a problem hiding this comment.
I think there is that .affectConfiguration function we should be using here. Or else we are clearing these out too much.
There was a problem hiding this comment.
No there isn't. Not on the config settings. That's on the workspaceConfiguration.
There was a problem hiding this comment.
And the worst case is that the code lens are regenerated whenever you change your python settings. Meh.
There was a problem hiding this comment.
You can see where the configSettings uses that 'affectsConfiguration' in the configSettings.ts file (line 571 or so). It will only fire an onDidChangeSettings when the python settings change.
In reply to: 407782405 [](ancestors = 407782405)
There was a problem hiding this comment.
We should probably recompute the code lenses though. Otherwise the next execution will recompute them all.
In reply to: 407782955 [](ancestors = 407782955,407782405)
| this.updateRequiredDisposable = this.codeLensFactory.updateRequired(this.onCodeLensFactoryUpdated.bind(this)); | ||
|
|
||
| // Make sure to stop listening for changes when this document closes. | ||
| this.documentManager.onDidCloseTextDocument(this.onDocumentClosed.bind(this)); |
There was a problem hiding this comment.
|
Kudos, SonarCloud Quality Gate passed!
|
* Eliminate first level of redundancy * Working with a cache * Add test to verify no more generating cell ranges * Add news entry * Clean up on closing * Make sure close and reopen works * Fix sonar errors * Fix restart * Fix wrong IDisposable type * Rename restartedKernel to onKernelRestarted * Recompute when changing settings * Clear out document close event when document closes
* Fix code lens perf for interactive window (#11142) * Eliminate first level of redundancy * Working with a cache * Add test to verify no more generating cell ranges * Add news entry * Clean up on closing * Make sure close and reopen works * Fix sonar errors * Fix restart * Fix wrong IDisposable type * Rename restartedKernel to onKernelRestarted * Recompute when changing settings * Clear out document close event when document closes * Update changelog * Fix build error from missing file
For #10971
Fix the perf of code lens generation by changing how we do a bunch of things.
Also got rid of the cellhashlogger as it was unnecessary. The cellHashProvider is a notebook execution logger. It doesn't need the in between piece.