Skip to content
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

avoid unnecessary scopes and variables calls #7313

Merged
merged 5 commits into from
Aug 27, 2021
Merged

avoid unnecessary scopes and variables calls #7313

merged 5 commits into from
Aug 27, 2021

Conversation

DavidKutu
Copy link

For #7246

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated.
  • Test plan is updated as appropriate.
  • package-lock.json has been regenerated by running npm install (if dependencies have changed).

@DavidKutu DavidKutu requested a review from a team as a code owner August 26, 2021 22:24
@DavidKutu DavidKutu requested a review from roblourens August 26, 2021 22:24
@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2021

Codecov Report

Merging #7313 (cfdbe2c) into main (c2b6335) will increase coverage by 0%.
The diff coverage is 0%.

@@          Coverage Diff           @@
##            main   #7313    +/-   ##
======================================
  Coverage     62%     63%            
======================================
  Files        360     360            
  Lines      22539   22551    +12     
  Branches    3404    3412     +8     
======================================
+ Hits       14080   14209   +129     
+ Misses      7284    7155   -129     
- Partials    1175    1187    +12     
Impacted Files Coverage Δ
src/client/debugger/jupyter/kernelDebugAdapter.ts 4% <0%> (-1%) ⬇️
src/client/datascience/activation.ts 86% <0%> (-6%) ⬇️
...atascience/jupyter/kernels/jupyterKernelService.ts 77% <0%> (-5%) ⬇️
src/client/common/cancellation.ts 72% <0%> (-4%) ⬇️
...ient/datascience/kernel-launcher/kernelLauncher.ts 90% <0%> (-2%) ⬇️
src/client/common/platform/fileSystem.ts 58% <0%> (-1%) ⬇️
.../localPythonAndRelatedNonPythonKernelSpecFinder.ts 79% <0%> (-1%) ⬇️
...lient/datascience/jupyter/kernels/cellExecution.ts 68% <0%> (-1%) ⬇️
...datascience/editor-integration/cellhashprovider.ts 64% <0%> (-1%) ⬇️
... and 22 more

@@ -377,7 +377,11 @@ export class KernelDebugAdapter implements DebugAdapter, IKernelDebugAdapter, ID
// Here we catch the stackTrace response and we use its id to send a scope message
if ((message as DebugProtocol.StackTraceResponse).command === 'stackTrace') {
(message as DebugProtocol.StackTraceResponse).body.stackFrames.forEach((sf) => {
this.scopes(sf.id);
// Only call scopes (and variables) if we are stopped on the cell we are executing
const cell = this.notebookDocument.cellAt(this.configuration.__cellIndex!);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this.configuration.__cellIndex always guaranteed to be defined here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not in all cases, I'll use the notebook URI

@@ -377,7 +377,11 @@ export class KernelDebugAdapter implements DebugAdapter, IKernelDebugAdapter, ID
// Here we catch the stackTrace response and we use its id to send a scope message
if ((message as DebugProtocol.StackTraceResponse).command === 'stackTrace') {
(message as DebugProtocol.StackTraceResponse).body.stackFrames.forEach((sf) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could even be optimized more - this will send the request for the cell's frame even if it is not the first frame in the stack, like when you have stepped into another cell. But it won't send it for other frames, which is good.

@DavidKutu DavidKutu merged commit dd31e31 into main Aug 27, 2021
@DavidKutu DavidKutu deleted the david/slowRBL branch August 27, 2021 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants