Bug 310992 - [vm][cache] Unnecessary cache flushing of thread children on suspended event
Summary: [vm][cache] Unnecessary cache flushing of thread children on suspended event
Status: NEW
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug-dsf (show other bugs)
Version: 7.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact: Jonah Graham CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-29 06:44 EDT by Anton Leherbauer CLA
Modified: 2020-09-04 15:26 EDT (History)
3 users (show)

See Also:
pawel.1.piech: review+


Attachments
Fix (1.12 KB, patch)
2010-04-29 06:50 EDT, Anton Leherbauer CLA
aleherb+eclipse: iplog-
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anton Leherbauer CLA 2010-04-29 06:44:08 EDT
I noticed that the DelayedStackRefreshUpdatePolicy flushes the cached data of the execution context for a suspended event which leads to a stack depth retrieval from the service when computing the delta for the event.
This should be avoided for optimal stepping performance.

See also Ken's mail on cdt-dev:
http://dev.eclipse.org/mhonarc/lists/cdt-dev/msg18566.html
Comment 1 Anton Leherbauer CLA 2010-04-29 06:50:36 EDT
Created attachment 166457 [details]
Fix

This seems to fix it.
Comment 2 Anton Leherbauer CLA 2010-04-29 06:51:45 EDT
Pawel, would you review before I commit?  I am not sure about potential side-effects.
Comment 3 Pawel Piech CLA 2010-04-29 19:11:20 EDT
The only other side effect that I can think of is that the thread's label and icon will not update until the full stack refresh event.  Having this update meant that the thread icon was often flashing between running and suspended states.  Eliminating this behavior is actually a positive change though.
Comment 4 Anton Leherbauer CLA 2010-04-30 08:34:36 EDT
(In reply to comment #3)
> The only other side effect that I can think of is that the thread's label and
> icon will not update until the full stack refresh event.  Having this update
> meant that the thread icon was often flashing between running and suspended
> states.  Eliminating this behavior is actually a positive change though.

I wonder what triggers the label update of the thread.  Is this a result of the EXPAND delta?

Related to that, would it make sense to have an update flag to update all properties?  This would correlate to the STATE delta.

Committed to HEAD.
Comment 5 Pawel Piech CLA 2010-04-30 14:20:21 EDT
(In reply to comment #4)
> I wonder what triggers the label update of the thread.  Is this a result of the
> EXPAND delta?
The resume event handling?  All this really begs for some controlled tests...
 

> Related to that, would it make sense to have an update flag to update all
> properties?  This would correlate to the STATE delta.
Yes!  It may even make sense to allow flushing of only select properties, at least that's the only way I can think of to implement bug 310992.
Comment 6 Marc Khouzam CLA 2010-05-03 14:39:37 EDT
I believe this fix caused Bug 311416 where stack frames are missing.  One may have to turn off the "Limit stack frames" option to see the problem.
Comment 7 Anton Leherbauer CLA 2010-05-20 03:18:02 EDT
The fix for this has caused issues, e.g. bug 312817, therefore I backed out the changes.
I am marking this invalid, because with the current cache behavior there is no way to relax the flushing rules.
Comment 8 Pawel Piech CLA 2010-05-20 13:23:46 EDT
(In reply to comment #7)
> The fix for this has caused issues, e.g. bug 312817, therefore I backed out the
> changes.
> I am marking this invalid, because with the current cache behavior there is no
> way to relax the flushing rules.

If it's OK with everyone, I'd like to keep this bug opened and try to address it again past 7.0.  We'll likely need to extend the cache API to support the optimal optimization ;-), but more importantly we'll need to have a good regression test suite also.
Comment 9 CDT Genie CLA 2010-07-28 15:24:49 EDT
*** cdt cvs genie on behalf of aleherbau ***
 Bug 310992 - [vm][cache] Unnecessary cache flushing of thread children on suspended event

[*] DelayedStackRefreshUpdatePolicy.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/ui/viewmodel/launch/DelayedStackRefreshUpdatePolicy.java?root=Tools_Project&r1=1.2&r2=1.3