Don't apply empty settings from not initialized TracingBlock

[iloveeclipse]

Fixes #1621

[iloveeclipse]

@HeikoKlare, @fedejeanne, @HannesWell : could you please check this PR and #1621?
For me it seem to fix the scenario I described in #1621.
However after so many fixes / reverts in this area the fix here seem to be too simple, so there must be a catch?
Do I miss something?

[iloveeclipse]

PS: my goal is to have some fix in 4.35 RC1 for #1621, whether it is this PR or revert of something again, or something completely different.

[github-actions]

Test Results

   285 files  ±0     285 suites  ±0   50m 51s ⏱️ + 5m 18s
 3 608 tests ±0   3 532 ✅ ±0   76 💤 ±0  0 ❌ ±0 
11 016 runs  ±0  10 785 ✅ ±0  231 💤 ±0  0 ❌ ±0 

Results for commit 60646cd. ± Comparison against base commit 79d7e75.

[HannesWell]

After a very quick look this change seems sound.
But I'll have a detailed review later this evening and will submit it, if it's fine and save.

[HannesWell]

After an in detail review this still looks save and the right thing to do.

[fedejeanne]

Hi, sorry for the late reply, I was out-of-office yesterday.

@iloveeclipse thank you for the fix. I actually run into a similar issue with the PluginsTab while working on #1252 and introduced a similar solution. The only difference I see with this PR is that you may initialize the tracing block with the same input when you switch back and forth between the tracing tab and other tabs. I avoided this to improve performance in https://github.com/eclipse-pde/eclipse.pde/pull/1252/files#diff-508c84ed339b4ddeadd5db68b30fc9a66cfd8db2c9ebd080678483782bbfaab7

Maybe that's also worth doing in your case?

[iloveeclipse]

The only difference I see with this PR is that you may initialize the tracing block with the same input when you switch back and forth between the tracing tab and other tabs. I avoided this to improve performance in https://github.com/eclipse-pde/eclipse.pde/pull/1252/files#diff-508c84ed339b4ddeadd5db68b30fc9a66cfd8db2c9ebd080678483782bbfaab7

Maybe that's also worth doing in your case?

Thanks for the hint.

I honestly have no time digging into performance optimization in this particular area, as I never observed any performance problems here nor anyone reported that for our product. If you know what to do, please provide a PR.

[HannesWell]

The only difference I see with this PR is that you may initialize the tracing block with the same input when you switch back and forth between the tracing tab and other tabs.

Can you explain this in more detail? The initialization of each tab from an existing launch-config should only happen once and at least the change in this PR doesn't change it. I assume you meant this as a general remark or do you think this is not save or wrong?

I honestly have no time digging into performance optimization in this particular area, as I never observed any performance problems here nor anyone reported that for our product. If you know what to do, please provide a PR.

The main pain-point here is that initializing the 'registry' of available tracing options scans all bundles in the TP and depending on it's size this can take some time. But it's only done once per Eclipse execution (which is actually wrong, because changing the TP may change the list of tracable bundles and each bundles options).
Maybe we should only postpone reading the registry and not the UI initialization (e.g. by using some kind of proxies) to avoid this problem in a more elegant way with less state. But all of this is something for #1252 I think. Sorry Federico I know you are awaiting a review and I'll try my best to do it soon.

[fedejeanne]

Can you explain this in more detail? The initialization of each tab from an existing launch-config should only happen once and at least the change in this PR doesn't change it.

That's correct and this PR doesn't change that so it's safe 👍 . The detailed explanation is:

• The initialization (call to org.eclipse.debug.ui.ILaunchConfigurationTab.initializeFrom(ILaunchConfiguration)) happens only once and it happens right at the beginning, when one selects the run configuration
• The activation (call to org.eclipse.debug.ui.ILaunchConfigurationTab.activated(ILaunchConfigurationWorkingCopy)) can happen any number of times (0+) and it happens every time the tab is selected

I assume you meant this as a general remark or do you think this is not save or wrong?

It was just a general remark about performance. This PR is safe and I don't see any need to add any changes to it. If performance every becomes a problem, we may add the early return (Zukunftsmusik).

The main pain-point here is that initializing the 'registry' of available tracing options scans all bundles in the TP and depending on it's size this can take some time. But it's only done once per Eclipse execution (which is actually wrong, because changing the TP may change the list of tracable bundles and each bundles options).
Maybe we should only postpone reading the registry and not the UI initialization (e.g. by using some kind of proxies) to avoid this problem in a more elegant way with less state. But all of this is something for #1252 I think. Sorry Federico I know you are awaiting a review and I'll try my best to do it soon.

I'd say we can leave the detailed discussion for #1252 :-)