Bug 536751 - Disable "show return value" preference by default to avoid hanging applications
Summary: Disable "show return value" preference by default to avoid hanging applications
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 4.7   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: JDT-Debug-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords:
Depends on:
Blocks:
 
Reported: 2018-07-06 07:08 EDT by Andrey Loskutov CLA
Modified: 2022-07-30 15:33 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Loskutov CLA 2018-07-06 07:08:00 EDT
Follow up on bug 533702 comment 7.

We see that "show return value ON" introduced and enabled by default via bug 40912 can show dramatic overhead (x800) compared with usual execution time and x25 compared with interpreted execution, so I propose to disable this feature by default, it is very dangerous.

I've personally already met cases where I had to kill debugger which was completely busy for a very long time due this option.

The problem for the end-users is that they have a 100% loaded debuggee without *any idea* that the problem is in the debugger and NOT in the application.
Comment 1 Till Brychcy CLA 2018-07-06 07:27:54 EDT
(In reply to Andrey Loskutov from comment #0)
> Follow up on bug 533702 comment 7.
> 
> We see that "show return value ON" introduced and enabled by default via bug
> 40912 can show dramatic overhead (x800) compared with usual execution time
> and x25 compared with interpreted execution, so I propose to disable this
> feature by default, it is very dangerous.

Just to get the number from that specific example right (I have no idea how representative it is)
Just stepping causes most of that overhead (factor more than x100)
The overhead of "Step Return" in that example leads to another factor of approx x7 (15860/2260ms)

I think simply disabling it is a bad idea.

- Part of it is that the exception being thrown is shown for an exception breakpoint which is very useful (and causes no extra slowdown), so there should be separate options for disabling the method return result only.
- In case we decide to disable it by default, it should be easy discoverable. Maybe a button for enabled it should be shown right as part of the variables view? Or the "Last result" entry should be always shown in the variables result, but possible disabled and in this case with a link that opens the preferences?

> 
> I've personally already met cases where I had to kill debugger which was
> completely busy for a very long time due this option.

I rarely had such "hanging" scenarios, but if, I never killed the debugger but just paused the thread and then did to a "Run-To-Line" to the point where I wanted to continue.

> 
> The problem for the end-users is that they have a 100% loaded debuggee
> without *any idea* that the problem is in the debugger and NOT in the
> application.

I agree that could be improved. 
BTW: The same kind of slow down is caused by Method Breakpoints, so we should try to find a solution which also warns when using these.
Comment 2 Sarika Sinha CLA 2018-07-08 23:29:30 EDT
I am fine with disabling this option by default, and yes we need to decide here what will be the best/easiest discover able option to provide.
Comment 3 Eclipse Genie CLA 2018-07-16 05:58:08 EDT
New Gerrit change created: https://git.eclipse.org/r/126093
Comment 4 Till Brychcy CLA 2018-07-17 15:16:32 EDT
I just had an idea and made an experiment with the example from bug 533702 comment 7:

I put a method exit breakpoint on random and ran (not stepped thru) the program- as expected the execution was slow. Then, without stopping the threads, I disabled this breakpoint during execution: Execution was completed quickly.

So we could do the following: We add a timeout setting (e.g. 3000 ms). If a step operation is started with "Show method results" on, and the timeout is exceeded, the breakpoints for observing method results are automatically disabled until the step operation is completed.
Comment 5 Andrey Loskutov CLA 2018-07-17 16:16:17 EDT
(In reply to Till Brychcy from comment #4)
> I just had an idea and made an experiment with the example from bug 533702
> comment 7:
> 
> I put a method exit breakpoint on random and ran (not stepped thru) the
> program- as expected the execution was slow. Then, without stopping the
> threads, I disabled this breakpoint during execution: Execution was
> completed quickly.
> 
> So we could do the following: We add a timeout setting (e.g. 3000 ms). If a
> step operation is started with "Show method results" on, and the timeout is
> exceeded, the breakpoints for observing method results are automatically
> disabled until the step operation is completed.

Patch?
Comment 6 Till Brychcy CLA 2018-07-18 06:18:25 EDT
(In reply to Till Brychcy from comment #4)
> I just had an idea and made an experiment with the example from bug 533702
> comment 7:
>
[...] 
> 
> So we could do the following: We add a timeout setting (e.g. 3000 ms). If a
> step operation is started with "Show method results" on, and the timeout is
> exceeded, the breakpoints for observing method results are automatically
> disabled until the step operation is completed.

I've created bug 537142 for this.
This should be easy to implement, I can probably make a patch for the functionality this evening (making a UI for configuration make take another evening).

This will also explain that this feature may be slowing down stepping when it gets triggered (which disabling the feature by default does not), so we should close the current bug als WONTFIX.

I think keeping the toolbar button Simeon made might sense but the feature should stay on by default, so we should create a separate bug for the toolbar button or rename the current bug.
Comment 7 Andrey Loskutov CLA 2018-07-21 09:05:19 EDT
(In reply to Till Brychcy from comment #6)
> (In reply to Till Brychcy from comment #4)
> > I just had an idea and made an experiment with the example from bug 533702
> > comment 7:
> >
> [...] 
> > 
> > So we could do the following: We add a timeout setting (e.g. 3000 ms). If a
> > step operation is started with "Show method results" on, and the timeout is
> > exceeded, the breakpoints for observing method results are automatically
> > disabled until the step operation is completed.
> 
> I've created bug 537142 for this.

Great. This makes the toolbar button and the change of the default obsolete.

> I think keeping the toolbar button Simeon made might sense but the feature
> should stay on by default, so we should create a separate bug for the
> toolbar button or rename the current bug.

I think we should apply now the first proposal from Simeon and add a *menu*, not button.
Comment 8 Eclipse Genie CLA 2022-07-30 15:33:02 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.