Skip to content

Manage the lifetime of DkmClrValue objects#343

Merged
amcasey merged 24 commits intodotnet:masterfrom
amcasey:DevDiv1094428
Feb 10, 2015
Merged

Manage the lifetime of DkmClrValue objects#343
amcasey merged 24 commits intodotnet:masterfrom
amcasey:DevDiv1094428

Conversation

@amcasey
Copy link
Member

@amcasey amcasey commented Feb 10, 2015

We need to attach the same information to DkmEvaluationResults and
DkmEvaluationResultEnumContexts, but they have different lifetimes. Enum
contexts, which are, in some sense, the children of evaluation results, are
closed first. As such, we don't want to clean up our attached information
until the evaluation result is closed. When we are informed that this has
happened (by a call to EvalResultDataItem.OnClose), we call Close on the
associated DkmClrValue.

We hope that actively managing the lifetime of DkmClrValue instances will
improve perf by reducing the amount of work that the debugger has to do on
each evaluation (which is proportional to the number of live
DkmClrValues).

Unfortunately, this will not improve perf in the large-array case because
DkmEvaluationResults are not closed as they are scrolled out of view.

Associated with DevDiv #1094428 (not yet on GitHub).

tmat and others added 13 commits February 8, 2015 17:19
This change deletes code we use to determine if a rule set file is one
that shipped with VS, and thus cannot be modified or deleted. It was
largely copied from code in
Microsoft.VisualStudio.CodeAnalysis.Sdk.UI.dll, and we should just use
the original instead.
NavigateTo doesn't support cancellation when computing the display properties so the cancellation token is getting removed rather than having to manually handling OperationCanceledException.
Fix dotnet#308 and dotnet#306 - issues with dynamic expressions in string interpolation
Remove cancellation token from NavigateTo
It is likely the character was inserted by accident. There is no reason
to use it in the code. It is a valid Unicode whitespace, but not a valid
ASCII. Since the containing file has no encoding bytemarks, the behavior
of this char is dependent on default encoding. Indeed, the file compiles
on Windows and fails on Linux.
As far as I could find, this is the only case where this character is
used in actual code (not in tests) in Roslyn codebase.
We need to attach the same information to DkmEvaluationResults and
DkmEvaluationResultEnumContexts, but they have different lifetimes.  Enum
contexts, which are, in some sense, the children of evaluation results are
closed first.  As such we don't want to clean up our attached information
until the evaluation result is closed.  When we are informed that this has
happened (by a call to EvalResultDataItem.OnClose), we call Close on the
associated DkmClrValue.

We hope that actively managing the lifetime of DkmClrValue instances will
improve perf by reducing the amount of work that the debugger has to do on
each evaluation (which is proportional to the number of live
DkmClrValues).

Unfortunately, this will not improve perf in the large-array case because
DkmEvaluationResults are not closed as they are scrolled out of view.

Associated with DevDiv #1094428 (not yet on GitHub).
@amcasey amcasey assigned KevinH-MS and unassigned KevinH-MS Feb 10, 2015
@amcasey
Copy link
Member Author

amcasey commented Feb 10, 2015

@CharlesStoner, you might be interested.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed a BOM.

Replace nonbreaking space (0xA0) with a regular space.
@amcasey
Copy link
Member Author

amcasey commented Feb 10, 2015

If you construct it in GetChildren, won't it be re-constructed for every call to GetChildren (e.g. when scrolling or collapsing and re-expanding)? This way, it's guaranteed to be constructed exactly once for each evaluation result.

tmat and others added 9 commits February 9, 2015 17:29
disable some tests due to interface tests.
Add MetadataVisualizer project to Roslyn solutions
fix vsct file to not have dupliate button definitions
while creating pull request, I left out this change
We need to attach the same information to DkmEvaluationResults and
DkmEvaluationResultEnumContexts, but they have different lifetimes.  Enum
contexts, which are, in some sense, the children of evaluation results are
closed first.  As such we don't want to clean up our attached information
until the evaluation result is closed.  When we are informed that this has
happened (by a call to EvalResultDataItem.OnClose), we call Close on the
associated DkmClrValue.

We hope that actively managing the lifetime of DkmClrValue instances will
improve perf by reducing the amount of work that the debugger has to do on
each evaluation (which is proportional to the number of live
DkmClrValues).

Unfortunately, this will not improve perf in the large-array case because
DkmEvaluationResults are not closed as they are scrolled out of view.

Associated with DevDiv #1094428 (not yet on GitHub).
amcasey added a commit that referenced this pull request Feb 10, 2015
Manage the lifetime of DkmClrValue objects
@amcasey amcasey merged commit ff272bd into dotnet:master Feb 10, 2015
AArnott pushed a commit to AArnott/roslyn that referenced this pull request Feb 14, 2015
@KevinH-MS KevinH-MS removed their assignment Mar 11, 2015
dibarbet pushed a commit to dibarbet/roslyn that referenced this pull request Nov 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.