Community
Participate
Working Groups
In Bug 441015 we added the new org.eclipse.ui.monitoring plug-in to platform.ui. This allow to report UI freezes. We should add it to the Eclipse IDE. AFAICT eclipse.platform.releng/org.eclipse.sdk is the right place to do this. Dani/David please correct me if I'm wrong.
A proposed patch is in https://git.eclipse.org/r/#/c/32181/.
It may be in the other bugs history, but can you summarize ... is this a new plugin? One that's been around for years, used occasionally by committers? I do not know about this particular plugin, but know just about any monitoring "think" is bound to add some overhead. In the past, there have been .debug options to "turn on" certain types of monitoring, but otherwise is 'off'. Is that the case here? If it is more a "committer tool" it should probably go in the "releng tool" bundle. One way to answer this question, of if it is a committer's tool, or a users tool, is "what would the user do with the data"? Also, you say it "is in UI" ... and if it is to detect "UI freezes" then that sounds like the right place for it (but ... maybe be over interpreting what you mean). Do you mean it's "in" current builds? Or, do you just mean it's a new bundle "in the git repo"? Overall, sounds pretty helpful, though. So don't take my questions as "negative".
(In reply to David Williams from comment #2) > It may be in the other bugs history, but can you summarize ... > > is this a new plugin? One that's been around for years, used occasionally by > committers? This is a new plugin based on the closed source one that has been used at Google for a year. We discovered many performance bugs in various parts of Eclipse Platform by using this plugin. > > I do not know about this particular plugin, but know just about any > monitoring "think" is bound to add some overhead. In the past, there have > been .debug options to "turn on" certain types of monitoring, but otherwise > is 'off'. Is that the case here? The monitoring is off by default. There is also a test that asserts that when the monitoring is on, it doesn't add more than 2.5% overhead to events that exceed the long event threshold. Overhead in absence of long events is negligible. > > If it is more a "committer tool" it should probably go in the "releng tool" > bundle. One way to answer this question, of if it is a committer's tool, or > a users tool, is "what would the user do with the data"? This is first and foremost an end user tool. When a user notices poor Eclipse responsiveness, she can use the monitoring information from the error log to file high quality bugs. Ideally, monitoring should be enabled ahead of time to provide insight into hard to reproduce UI freezes. > > Also, you say it "is in UI" ... and if it is to detect "UI freezes" then > that sounds like the right place for it (but ... maybe be over interpreting > what you mean). Do you mean it's "in" current builds? Or, do you just mean > it's a new bundle "in the git repo"? The plugin is being built and tested as part of platform UI builds. > > Overall, sounds pretty helpful, though. So don't take my questions as > "negative".
(In reply to Sergey Prigogin from comment #3) >> If it is more a "committer tool" it should probably go in the "releng tool" >> bundle. One way to answer this question, of if it is a committer's tool, or >> a users tool, is "what would the user do with the data"? > This is first and foremost an end user tool. When a user not>ices poor Eclipse > responsiveness, she can use the monitoring information from the error log to > file high quality bugs. Ideally, monitoring should be enabled ahead of time to > provide insight into hard to reproduce UI freezes. I hope to look closer at this soon, but wanted to clarify "terminology". From your description, this is "committer tool" if it's main purpose it to provide better bug reports. As I use the terminology, an "user tool" are those that warn users with something like "animation appears to be taking a large percentage of CPU on your machine, you may want to consider turning that off, for a better overall experience". And of course, I just picked "animation" as as example I've seen in other programs .... with Eclipse, it'd be more likely to be "as you type XML Validation" or "Having every compiler issue marked as a warning, instead of 'ignoring' some of them". And that's not a criticism, just a clarification on how I use the terminology.
two minor comments: 1. Similar to the error reporting tool I'm working on for Mars Milestones, it may make sense to just add it to several EPP packages rather than adding it to the platform. I think the standard IDE user is not necessarily the person you target but Eclipse Committers and milestone testers. There, however, I'd enable it by default. But here others will have other opinions. 2. I'd like to see the plugin creating error log entries in the case of "severe" timing issues. The reason for that is that we automatically collect error reports only. We listen to warnings as well but then plug-in id, error code should to be stable so that we really report severe issues. My 2 cents.
(In reply to Marcel Bruch from comment #5) > 2. I'd like to see the plugin creating error log entries in the case of > "severe" timing issues. The reason for that is that we automatically collect > error reports only. We listen to warnings as well but then plug-in id, error > code should to be stable so that we really report severe issues. What is your definition of "severe" issues?
(In reply to Sergey Prigogin from comment #6) > (In reply to Marcel Bruch from comment #5) > > 2. I'd like to see the plugin creating error log entries in the case of > > "severe" timing issues. The reason for that is that we automatically collect > > error reports only. We listen to warnings as well but then plug-in id, error > > code should to be stable so that we really report severe issues. > > What is your definition of "severe" issues? UI freeze in sense of having the "beach ball / wheel of death" [1] shown is severe for me. Freezes over 5 seconds and deadlocks are really severe IMHO. [1] http://en.wikipedia.org/wiki/Spinning_pinwheel
Just to give a little warning ... I think this is staring to look "iffy" for M2. Seems like there are still some questions/issues with it, and while I do want to still "take a close look", thought I'd post an little warning here I'm less certain about it making M2 than I was a week ago. But, do still agree it's important.
(In reply to Marcel Bruch from comment #5) > two minor comments: > > 1. Similar to the error reporting tool I'm working on for Mars Milestones, > it may make sense to just add it to several EPP packages rather than adding > it to the platform. I think the standard IDE user is not necessarily the > person you target but Eclipse Committers and milestone testers. There, > however, I'd enable it by default. But here others will have other opinions. In the Eclipse SDK it will not be enabled by default. > 2. I'd like to see the plugin creating error log entries in the case of > "severe" timing issues. The reason for that is that we automatically collect > error reports only. We listen to warnings as well but then plug-in id, error > code should to be stable so that we really report severe issues. See Bug 443572
(In reply to David Williams from comment #8) > Just to give a little warning ... I think this is staring to look "iffy" for > M2. > > Seems like there are still some questions/issues with it, and while I do > want to still "take a close look", thought I'd post an little warning here > I'm less certain about it making M2 than I was a week ago. I prefer to have it integrated as soon as possible. It is disabled by default and interested parties could enable it and help us to improve the interactive performance for Mars.
There seems to be confusion on the "target user" for this plug-in. This plug-in is valuable to multiple audiences. Our reasons for creating it are: 1) To improve the responsiveness of the Eclipse platform and associated frameworks. In that vein, it is of great value to platform/JDT/CDT/Xtext/etc. developers and early "dogfooders" who live on the bleeding edge. I firmly believe that bug 441726 would have never made it into the 4.4.0 release if this plug-in had been in place. In fact our intern uploaded a screenshot that exposed that bug when he filed bug 441015. That was not planned. Currently some additional (hopefully minor) tuning is necessary. We don't have FILTER_TRACES defined for the standard OSes. For Linux there are two: one for OS.gtk_dialog_run and another for DnDManager.startDrag. I strongly encourage developers in the Eclipse ecosystem enable this plug-in and help tune it to make it a source of highly relevant data. 2) To fix responsiveness problems in Google's products (e.g., the Google Plug-in for Eclipse) and to help us support the end users of our products. We can ask external users to enable this plug-in and send us their standard error logs. For Google-internal users, we enable it by default and automatically collect the error logs whenever a user reports an error. The plug-in also provides an extension point for subscribing to UI freeze/deadlock notifications. Those notifications can be dealt with in product-specific or installation-specific ways. For the GPE, we may filter notifications based on whether they involve the GPE, prepare a bug report and prompt the user to file the bug. For internal Google users, we will log all UI freezes to a central repository for aggregate analysis. 3) To enable the general population of Eclipse users to help move the Eclipse ecosystem forward. Currently it requires "advanced" knowledge (and lucky timing if delays are < 2 seconds) to run jstack to extract data on UI responsiveness. This plug-in can put high-quality UI freeze/deadlock data directly into the error log, so anyone who is familiar with attaching the error log when filing a bug report can contribute valuable information that helps make Eclipse more responsive.
(In reply to Terry Parker from comment #11) > There seems to be confusion on the "target user" for this plug-in. This > plug-in is valuable to multiple audiences. Our reasons for creating it are: > > 1) To improve the responsiveness of the Eclipse platform and associated > frameworks. In that vein, it is of great value to > platform/JDT/CDT/Xtext/etc. developers and early "dogfooders" who live on > the bleeding edge. > > I firmly believe that bug 441726 would have never made it into the 4.4.0 > release if this plug-in had been in place. In fact our intern uploaded a > screenshot that exposed that bug when he filed bug 441015. That was not > planned. > > Currently some additional (hopefully minor) tuning is necessary. We don't > have FILTER_TRACES defined for the standard OSes. For Linux there are two: > one for OS.gtk_dialog_run and another for DnDManager.startDrag. I strongly > encourage developers in the Eclipse ecosystem enable this plug-in and help > tune it to make it a source of highly relevant data. > > 2) To fix responsiveness problems in Google's products (e.g., the Google > Plug-in for Eclipse) and to help us support the end users of our products. #2 was written specifically about Google's products, but hopefully everyone sees this applies more broadly to any group that provides Eclipse-based products or tooling. The strategies we are adopting for dealing with with both internal and external support can be adopted by anyone. > > We can ask external users to enable this plug-in and send us their standard > error logs. For Google-internal users, we enable it by default and > automatically collect the error logs whenever a user reports an error. > > The plug-in also provides an extension point for subscribing to UI > freeze/deadlock notifications. Those notifications can be dealt with in > product-specific or installation-specific ways. For the GPE, we may filter > notifications based on whether they involve the GPE, prepare a bug report > and prompt the user to file the bug. For internal Google users, we will log > all UI freezes to a central repository for aggregate analysis. > > 3) To enable the general population of Eclipse users to help move the > Eclipse ecosystem forward. > > Currently it requires "advanced" knowledge (and lucky timing if delays are < > 2 seconds) to run jstack to extract data on UI responsiveness. This plug-in > can put high-quality UI freeze/deadlock data directly into the error log, so > anyone who is familiar with attaching the error log when filing a bug report > can contribute valuable information that helps make Eclipse more responsive.
Here's some quick comments and questions ... - It's said "...Currently some additional (hopefully minor) tuning is necessary...". Who is signed up to do that? How long would you expect that to take? Another milestone? or two? (In other words, more specifically, besides "providing the code" the original authors, from Google, plan on continuing to maintain and support it ... right? [And, apologize for my ignorance of "UI Committers" but is the author already a UI committer? an e4 committer?] - There is a org.eclipse.ui.monitoring.tests bundle, I see. Where does that go? It appears to be a fragment, that is intended to run during the build? With surefire? Are there plans to get that working, somehow? Converted to our "standard" test framework? [While a separate topic, it'd be icing on the cake to have this running during our unit tests, right? Or, at least the ability to have it run, during some runs. I've not thought that through, completely, but would seem to be both a source of "early warning", as well as a help to "fine tune" the parameters?] - The bundle is named "Event Loop Monitor". Off hand, that sounds pretty general. Is that intentional? (That is, is it really that general?). I was expecting something like "Display Loop Monitor" or "UI Event Loop Monitor" ... so, thought I'd ask to increase my understanding. - Easiest of all, currently has Bundle-Vendor=Google Inc. but that should be changed to "Eclipse.org" as other Eclipse Platform bundles have. Similar for "name" of test bundle ... "Google Eclipse Monitoring Tests" should be more "vendor neutral" and be named "Event Loop Monitor Tests", or similar. - At some point, this will need some "user/isv doc", I'm guessing ... is author willing to provide that? Within a milestone or two?
(In reply to David Williams from comment #13) > Here's some quick comments and questions ... > - One I forgot to mention. This currently appears to be "ran at startup", just going by plugin.xml. Is that really necessary? What does it do at startup? As I'm sure you know, we try to avoid bundles "doing things" at startup, so again, thought I'd ask for explanation, rather then study the code. And, though have mentioned not to take any of my questions as negative. I still see as valuable and important, and appreciate the contribution. Thanks,
(In reply to David Williams from comment #13) > Here's some quick comments and questions ... > > - It's said "...Currently some additional (hopefully minor) tuning is > necessary...". Who is signed up to do that? How long would you expect that > to take? Another milestone? or two? (In other words, more specifically, > besides "providing the code" the original authors, from Google, plan on > continuing to maintain and support it ... right? [And, apologize for my > ignorance of "UI Committers" but is the author already a UI committer? an e4 > committer?] Google is committed to doing that. That is mentioned in https://bugs.eclipse.org/bugs/show_bug.cgi?id=441015#c20. My phrasing is there because we don't typically run Mac or Windows, so that is an area where other developers can help to ensure the correct set of filters is in place. > > - There is a org.eclipse.ui.monitoring.tests bundle, I see. Where does that > go? > It appears to be a fragment, that is intended to run during the build? With > surefire? Are there plans to get that working, somehow? Converted to our > "standard" test framework? [While a separate topic, it'd be icing on the > cake to have this running during our unit tests, right? Or, at least the > ability to have it run, during some runs. I've not thought that through, > completely, but would seem to be both a source of "early warning", as well > as a help to "fine tune" the parameters?] I thought that that already ran as part of automated Maven tests, but I'll need to check to make sure. That is the goal. Do you not see it running? > > - The bundle is named "Event Loop Monitor". Off hand, that sounds pretty > general. Is that intentional? (That is, is it really that general?). I was > expecting something like "Display Loop Monitor" or "UI Event Loop Monitor" > ... so, thought I'd ask to increase my understanding. In the code the thread is being named "[DEBUG] SWT Event watchdog, analysis, and logging thread". The bundle name is likely just an oversight. > > - Easiest of all, currently has > Bundle-Vendor=Google Inc. > but that should be changed to "Eclipse.org" as other Eclipse Platform > bundles have. Similar for "name" of test bundle ... "Google Eclipse > Monitoring Tests" should be more "vendor neutral" and be named "Event Loop > Monitor Tests", or similar. > > - At some point, this will need some "user/isv doc", I'm guessing ... is > author willing to provide that? Within a milestone or two? Yes, we will fix this.
(In reply to David Williams from comment #14) > (In reply to David Williams from comment #13) > > Here's some quick comments and questions ... > > > > - One I forgot to mention. This currently appears to be "ran at startup", > just going by plugin.xml. Is that really necessary? What does it do at > startup? As I'm sure you know, we try to avoid bundles "doing things" at > startup, so again, thought I'd ask for explanation, rather then study the > code. It needs to run at startup so that the "[DEBUG] SWT Event watchdog, analysis, and logging" thread can be launched if monitoring is enabled. That is the only thing it does. > > And, though have mentioned not to take any of my questions as negative. I > still see as valuable and important, and appreciate the contribution. > > Thanks,
This work is based off of a plug-in we have been deploying inside of Google for over a year. In open-sourcing it we did rewrite parts of it to make it more flexible and are in the process of redeploying our internal installation to use this plug-in rather than our previous in-house version. I expect to have it redeployed within the next couple of weeks. We rely heavily upon this monitoring (our environment and users are extremely sensitive to UI responsiveness) so we will be actively working on tuning it.
(In reply to David Williams from comment #13) > Here's some quick comments and questions ... Thanks David for the feedback and questions. > - It's said "...Currently some additional (hopefully minor) tuning is > necessary...". Who is signed up to do that? How long would you expect that > to take? Another milestone? or two? (In other words, more specifically, > besides "providing the code" the original authors, from Google, plan on > continuing to maintain and support it ... right? [And, apologize for my > ignorance of "UI Committers" but is the author already a UI committer? an e4 > committer?] Dani Megert and I agreed that I would be the responsible committer for this development. My target is to nominate Sergey as committer once we have sufficient contributions. > - There is a org.eclipse.ui.monitoring.tests bundle, I see. Where does that > go? > It appears to be a fragment, that is intended to run during the build? With > surefire? Are there plans to get that working, somehow? Converted to our > "standard" test framework? [While a separate topic, it'd be icing on the > cake to have this running during our unit tests, right? Or, at least the > ability to have it run, during some runs. I've not thought that through, > completely, but would seem to be both a source of "early warning", as well > as a help to "fine tune" the parameters?] The Maven run works already. It is not yet automatically included in the Tycho build (like almost no plug-in) If you need something else for the "standard" test framework, please open a new bug and copy Sergey and myself into it. > > - The bundle is named "Event Loop Monitor". Off hand, that sounds pretty > general. Is that intentional? (That is, is it really that general?). I was > expecting something like "Display Loop Monitor" or "UI Event Loop Monitor" > ... so, thought I'd ask to increase my understanding. > - Easiest of all, currently has > Bundle-Vendor=Google Inc. > but that should be changed to "Eclipse.org" as other Eclipse Platform > bundles have. Similar for "name" of test bundle ... "Google Eclipse > Monitoring Tests" should be more "vendor neutral" and be named "Event Loop > Monitor Tests", or similar. Created Bug 443663 for this. > - At some point, this will need some "user/isv doc", I'm guessing ... is > author willing to provide that? Within a milestone or two? Created Bug 443664. I would suggest to provide this documentation +2 milestones after we integrated the tool into the SDK and would expect that Sergey or Terry provide it. I already created Bug 442354 for the N&N entry which should IMHO provided once the plug-in gets integrated into the SDK.
(In reply to Lars Vogel from comment #18) > > Dani Megert and I agreed that I would be the responsible committer for this > development. My target is to nominate Sergey as committer once we have > sufficient contributions. > Thanks for clarifying. Another question for you then: was there a CQ filed for this code, I assume originally attached to a bugzilla? Sorry to review "Eclipse 101" stuff that's likely been covered already, but on the surface appears to be well over 1000 lines of code ... so, thought I should ask.
I will comment on one general concern, and then allow others to correct me and/or offer suggestions. From what I've seen of preference page, the "usefulness" of this type of monitoring depends greatly on having the correct values set, there in the preference pages. Where "correct" is actually a science unto itself. What I mean by that -- and please correct me if I am simply making the wrong assumptions -- is that there is no "averaging" taking place, no "adaptive behavior" ... time is over the criteria: log and file bug. Right? AND the reason that concerns me, is I can imagine that in the environment that has "seen the most use" all the developers have relatively similar class of machines ... relatively well equipped, relatively modern, relatively uniform. And in that environment, "static criteria" likely works fine. But, in the "world of Eclipse", I've always imagined a very diverse set of machines. Some of them really tricked out, some of them really really old, like "XP on dual core" processor (half a joke). So, that's fine and well, for Eclipse users BUT, the reason that concerns me, is that unless users really know how to set those criteria, we could easily end up with "hundreds of bug reports" which, basically, were generated because the static criteria didn't match the machine's capabilities. So, someone (who?) must spend hours and hours pouring through those hundreds of bugs, trying to see if any "make sense" (i.e. point to a problem to improve in code) and if others "don't make sense" (i.e. really just reflect the capability of the machine and perhaps all the other software running on it, and the initial settings being set incorrectly). To state my concern another way, those with really fast machines, may seldom see logs generated, even though Eclipse seems slower than other software they use, and those with older, less capable machines, might see lots of "errors" logged, even though Eclipse seems just as fast or faster than other software they use. Any parts of that wordy observation totally off the mark? If not, Questions are a) how to know wheat from chafe? Any analysis tools? Heuristics? Tricks of the trade? b) Who's going to do that work of triaging "hundreds of bugs"? If we get that many, of course. (It would be a bad thing for us to get hundreds of such bug reports, and then not do anything with them). If my naive observations are correct, I'd encourage us all to present this (such as in New and Noteworthy) as "experimental, and may or may not be in final release". = = = = = = And, please, correct your terminology, even though bug title is correct, many people refer to this as "putting this bundle in the SDK" but in fact you are proposing to "put this in the Platform" -- BIG difference. (not least of which, is almost no one "includes" the SDK in a "product" (such as EPP packages) but nearly everyone includes the Platform. Thanks again, Sorry to be so wordy.
Oh, and, on the tests, I suggest you just put <properties> <skipTests>false</skipTests> </properties> in the POM, and see what happens. (For ?all? other "test bundles" they are written to run in our "post build" test run ... and since this one is not, I think it should be turned on during the build, and assume they are simple, nearly always pass "unit tests" (not "integration tests" as many of our other tests are). Thanks again,
(In reply to David Williams from comment #19) > (In reply to Lars Vogel from comment #18) > Thanks for clarifying. Another question for you then: was there a CQ filed > for this code, I assume originally attached to a bugzilla? Yes, thanks for validating, I was lucky that Dani reminded me about that. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=441015#c22 and https://dev.eclipse.org/ipzilla/show_bug.cgi?id=8571
(In reply to David Williams from comment #20) > Who's going to do that work of triaging "hundreds of bugs"? The feature will be disabled by default, so only an informed person will know how to activate it. Given my experience with Eclipse consumers and their willingness to report bugs I don't expert to many bug reports. Having this early in Mars should also help to find the most annoying things before the end user might start using it. > Best values We can tweak them once we get feedback during the Mars release cycle. > = = = = = = > > And, please, correct your terminology, even though bug title is correct, > many people refer to this as "putting this bundle in the SDK" but in fact > you are proposing to "put this in the Platform" -- BIG difference. (not > least of which, is almost no one "includes" the SDK in a "product" (such as > EPP packages) but nearly everyone includes the Platform. I think in this case the terminology is correct. We already have it in the platform (Git repository). See Bug 441015. So this Bug is really about adding it to the SDK (via the feature).
(In reply to David Williams from comment #21) > Oh, and, on the tests, I suggest you just put > > <properties> > <skipTests>false</skipTests> > </properties> > > in the POM, and see what happens. We still fighting some issues with running the tests, I currently have only integrated one plug-in into it. See Bug 443396 and Bug 443668. But I created Bug 443678 to track this activation.
(In reply to Lars Vogel from comment #23) > (In reply to David Williams from comment #20) > I think in this case the terminology is correct. We already have it in the > platform (Git repository). See Bug 441015. So this Bug is really about > adding it to the SDK (via the feature). platform != git repository platform = platform feature, or platform product, or platform binary distribution on download page ... ... at least, as I use the term ... which matches what the Gerrit patch is. If it is already in "the platform" (as I am using the term), it would automatically be in the SDK. You probably mean the "Eclipse Platform Project", which is actually an unofficial name ... it's official name is simply the "Eclipse Project". But don't miss the significance of my comment. If it is in the platform feature, it ends up in "nearly everything" built on Eclipse (well, every "ide sort of thing"). If it was in only the Eclipse SDK, then it would end up in very few products, unless those products explicitly included it. If the intent is to have in only the Eclipse SDK distribution, and not the Platform feature -- thus giving products the choice of if they want it or not (sort of a good idea?) -- then I think the patch is the wrong one, and I've been misunderstanding all along.
(In reply to David Williams from comment #25) > But don't miss the significance of my comment. If it is in the platform > feature, it ends up in "nearly everything" built on Eclipse (well, every > "ide sort of thing"). If it was in only the Eclipse SDK, then it would end > up in very few products, unless those products explicitly included it. It is disabled by default but having this available in the platform feature allows Eclipse developers to ask their users to provide better error reports. I think that is an important service the platform team should provide. I see it similar to the "Tracing" capabilities in platform which is also included in the platform.
(In reply to Lars Vogel from comment #26) > (In reply to David Williams from comment #25) > > > But don't miss the significance of my comment. If it is in the platform > > feature, it ends up in "nearly everything" built on Eclipse (well, every > > "ide sort of thing"). If it was in only the Eclipse SDK, then it would end > > up in very few products, unless those products explicitly included it. > > It is disabled by default but having this available in the platform feature > allows Eclipse developers to ask their users to provide better error > reports. I think that is an important service the platform team should > provide. > > I see it similar to the "Tracing" capabilities in platform which is also > included in the platform. I am not disagreeing! Just wanted to be sure it was talked about clearly. So "everyone would know they were getting it". In fact, I've CC'd Markus Knauer, "lead of EPP" -- simply for awareness -- since so many of the EPP package start include the Platform feature. (I don't think there is anything for you to do, Markus, just wanted to be sure you were aware of this new function, that will be better described in New and Noteworthy. It also recently dawned on me as people "fine tune" what values to recommend, they should use some of the "larger" packages, such as JEE, or CDT. It may be found different values are best, or perhaps "the most conservative" would want to be chosen as the default. Again, I'm half guessing "how this works", but I would assume if you had a context menu with 25 entries contributed, it would naturally take longer than a context menu with 5 entries contributed, so would hate for those with 25 entries to constantly get messages "pop up menu took 5 time longer than criteria". Hope I'm making sense ... and I'm not complaining :) just trying to give some advice.
And, I know I'm really off-topic of the purpose of this bug, but ... I'm also CC'ing Tom Watson, to see if there is any better recommendation on how to handle the "early startup" nature of the bundle. Part of what bothers me is that then basically two "preferences" come into play. Someone may "turn of" the early startup of the bundle, in which case the actual "monitor" preference would no longer work. Right? I wonder if there is a better model ... something like the way "content type" handlers are done. There, the bundle is still marked "lazy activation" EXCEPT for one class, or package. Just enough to see if the preference is set, and if preference is not set, nothing else happens and the bundle is never loaded. But, again, I'm half guessing, which is why I'm "asking the experts" :) Perhaps the time for early startup is so minor, it is not worth thinking about, but I know especially the larger packages, such as JEE, and others have lots of "early startup" bundles. At least in this case, the other bundles it might "startup" as a side effect ("prefernces, UI, jface", are all bundles that pretty much have to startup early anyway (unlike the "content type" bundles, where for example, if you never work with HTML, it's more of a waste to start up a bundle whose purpose is to work with HTML). Again ... I'm way off topic, but if any of my ideas help in the slightest way, I'd be happy I could help ... but, I don't mean in any way these are things that "have" to be done.
BTW, I would like to get at least the provider name fixed, if not also the plugin names, before putting in a build. I hate to be picky ... but, I'm surprised it hasn't been done already, so thought I'd better mention I was waiting for that.
(In reply to David Williams from comment #29) > BTW, I would like to get at least the provider name fixed, if not also the > plugin names, before putting in a build. I hate to be picky ... but, I'm > surprised it hasn't been done already, so thought I'd better mention I was > waiting for that. I marked this bug as dependent on Bug 443663.
(In reply to David Williams from comment #28) > And, I know I'm really off-topic of the purpose of this bug, but ... > > I'm also CC'ing Tom Watson, to see if there is any better recommendation on > how to handle the "early startup" nature of the bundle. Part of what bothers > me is that then basically two "preferences" come into play. Someone may > "turn of" the early startup of the bundle, in which case the actual > "monitor" preference would no longer work. Right? > In order to answer the question I need to understand what this bundle is doing in its BundleActivator.start method. If the answer is nothing, then there is no reason to have it an early startup bundle.
(In reply to David Williams from comment #20) > From what I've seen of preference page, the "usefulness" of this type of > monitoring depends greatly on having the correct values set, there in the > preference pages. Where "correct" is actually a science unto itself. > > What I mean by that -- and please correct me if I am simply making the wrong > assumptions -- is that there is no "averaging" taking place, no "adaptive > behavior" ... time is over the criteria: log and file bug. Right? > > AND the reason that concerns me, is I can imagine that in the environment > that has "seen the most use" all the developers have relatively similar > class of machines ... relatively well equipped, relatively modern, > relatively uniform. And in that environment, "static criteria" likely works > fine. But, in the "world of Eclipse", I've always imagined a very diverse > set of machines. Some of them really tricked out, some of them really really > old, like "XP on dual core" processor (half a joke). So, that's fine and > well, for Eclipse users > > BUT, the reason that concerns me, is that unless users really know how to > set those criteria, we could easily end up with "hundreds of bug reports" > which, basically, were generated because the static criteria didn't match > the machine's capabilities. > > So, someone (who?) must spend hours and hours pouring through those hundreds > of bugs, trying to see if any "make sense" (i.e. point to a problem to > improve in code) and if others "don't make sense" (i.e. really just reflect > the capability of the machine and perhaps all the other software running on > it, and the initial settings being set incorrectly). > > > To state my concern another way, those with really fast machines, may seldom > see logs generated, even though Eclipse seems slower than other software > they use, and those with older, less capable machines, might see lots of > "errors" logged, even though Eclipse seems just as fast or faster than other > software they use. > > Any parts of that wordy observation totally off the mark? David, thanks for airing your concerns. We do need to make sure that when Mars is launched, UI freeze monitoring doesn’t spam Eclipse users with irrelevant data, which as you point out could create an influx of useless bug reports would burden the community. There are two ways to handle this concern: 1) Leave the freeze monitoring disabled by default, as it currently is. 2) Make sure the "Long event threshold" for logging freeze events is sufficiently high that users running on the "minimum recommended hardware" are not spammed with meaningless reports. As for "tuning" the settings, there are only two settings that can affect the quantity of freeze events generated. 1) The "Long event threshold". A smaller threshold means shorter events get logged. At the same threshold, previous generation hardware may potentially log more events than current hardware. These will be actual UI responsiveness issues for that user on that hardware--the choices are to lower the bar for all hardware or try to do something adaptive. But we don't have actual data yet that shows this is an issue. 2) The "Stack Traces to Filter Out". Each OS has a set of signatures that may show up in freeze events when in fact the UI is being responsive (e.g., OS.gtk_dialog_run). In the end it doesn't appear that the tuning of UI freeze monitoring is going to be fussy. Bug 443346 made things appear really bad, it caused bogus freeze events to get published, especially when the logging threshold was set to less than one second. With that patch installed I ran Eclipse all afternoon with a logging threshold of 1000ms and saw no bogus freeze events, whereas without the patch with a 500ms threshold the signal-to-noise ratio was terrible. We apologize for that bug, it was a bad oversight. You bring up a good point about many Eclipse developers running on higher end workstations. At home I have a 5+ year old machine that I never fire up anymore running I-don't-even-remember version of MS Windows. I'll be glad to do some testing there after the end of the quarter and report back the machine specs and results. If a few others are willing to do the same we might come up with a good baseline. Given that we are over 9 months away from the Mars release, we have a large window to get experience with UI freeze monitoring and make the right decisions. > > If not, Questions are > > a) how to know wheat from chafe? Any analysis tools? Heuristics? Tricks of > the trade? > > b) Who's going to do that work of triaging "hundreds of bugs"? If we get > that many, of course. (It would be a bad thing for us to get hundreds of > such bug reports, and then not do anything with them). Bug 443346 caused a lot of chaff, but we haven't yet established that there are problems beyond that. I don't expect you to take my word for it--let's get some more experience with it and see. If we can't get a high signal-to-noise ratio for users with older machines, then I'd agree that doesn't make sense enable UI freeze monitoring for the general population (within reason, if a machine struggles with running Eclipse at all then there is no point optimizing for that.) > > If my naive observations are correct, I'd encourage us all to present this > (such as in New and Noteworthy) as "experimental, and may or may not be in > final release". > > = = = = = = > > And, please, correct your terminology, even though bug title is correct, > many people refer to this as "putting this bundle in the SDK" but in fact > you are proposing to "put this in the Platform" -- BIG difference. (not > least of which, is almost no one "includes" the SDK in a "product" (such as > EPP packages) but nearly everyone includes the Platform. > > Thanks again, > Sorry to be so wordy.
(In reply to Thomas Watson from comment #31) > (In reply to David Williams from comment #28) > > And, I know I'm really off-topic of the purpose of this bug, but ... > > > > I'm also CC'ing Tom Watson, to see if there is any better recommendation on > > how to handle the "early startup" nature of the bundle. Part of what bothers > > me is that then basically two "preferences" come into play. Someone may > > "turn of" the early startup of the bundle, in which case the actual > > "monitor" preference would no longer work. Right? > > > > In order to answer the question I need to understand what this bundle is > doing in its BundleActivator.start method. If the answer is nothing, then > there is no reason to have it an early startup bundle. The plug-in doesn't do much in its activator's start() method, but early startup is used to start the UI monitoring thread. Is there a preferred way to do that other than via early startup? See: https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org.eclipse.ui.monitoring/src/org/eclipse/ui/internal/monitoring/MonitoringStartup.java
(In reply to Thomas Watson from comment #31) > (In reply to David Williams from comment #28) > > And, I know I'm really off-topic of the purpose of this bug, but ... > > > > I'm also CC'ing Tom Watson, to see if there is any better recommendation on > > how to handle the "early startup" nature of the bundle. Part of what bothers > > me is that then basically two "preferences" come into play. Someone may > > "turn of" the early startup of the bundle, in which case the actual > > "monitor" preference would no longer work. Right? > > > > In order to answer the question I need to understand what this bundle is > doing in its BundleActivator.start method. If the answer is nothing, then > there is no reason to have it an early startup bundle. I was going to try and summarize for you, but by my casual reading of the start method code, it actually is doing something with "debug options", not "preferences" as I initially thought ... so, I'm either mis-reading it ... or, just will take some study time and/or explanation (for someone like me to understand it). [Just saw Terry Parker's response as I was posting this, so will explain a little more ... maybe I'm looking at the wrong plugin :) but in 'start' method, I see a "Tracer.create(....)" method call, and in that method I see if (isTracingEnabled(debugOption)) and isTracingEnabled is basically ... public static boolean isTracingEnabled(String debugOption) { return Platform.isRunning() && Boolean.parseBoolean(Platform.getDebugOption(debugOption)); } None of this "looks bad" to me (except, not sure if Platform.isRunning() is safe in a start sequence? I'd have to look at what it does.) ... but, it just does not look related to "preferences" as I thought, but instead debug options. So, like I said, will take more study or explanation.
Ok, much to the unhappiness of many followers of this bug, I have decided. I think we should put this in right after M2 comes out, and get some experience with it in N-builds and I-builds before it arrives in a milestone. (Which, BTW, is pretty much always true ... and I was just considering an exception for this case since many trusted people seemed to feel so strongly about it). My decision is in no way a "criticism" and does not change my opinion this is important and worthwhile, but it is more related to the general "releng best practice" of not putting brand new things in the final few days before a milestone. I don't know about you ... but, I've had enough stress this week ... so have become especially risk averse. I think putting in after the milestone is ideal, since then the "core committers" in Platform can turn it on, and get some early testing, before we increase the audience size of a milestone. Put another way, I don't think this will slow down the work that's needed with tuning, etc. ... since Platform committers (and other early adopters) work a lot with I-builds. Whereas an M-build, especially being part of EPP packages, may be a little more audience than we'd like, to start off with. So, sorry to disappoint ... but, trust me. We'll be better off for it!
(In reply to David Williams from comment #35) > Ok, much to the unhappiness of many followers of this bug, I have decided. > > I think we should put this in right after M2 comes out, and get some > experience with it in N-builds and I-builds before it arrives in a > milestone. Awesome news to have soon in the build! Thanks. I also agree that we should have this in an early build.
I agree that "right after M2" is the best time for integration. I measured the time spent in earlyStartup and found it to be about 9ms when monitoring is off. (Filed bug 443809 for a far worse offender.) Note that the earlyStartup doesn't "just read a preference". As side effects, it also starts the MonitoringPlugin and runs the MonitoringPreferenceInitializer. Unfortunately, I don't think there's a better way for a self-contained plug-in. An alternative could be to put the preference into an existing UI bundle and check the preference when that bundle starts.
I've applied the Gerrit patch (after rebasing). http://git.eclipse.org/c/platform/eclipse.platform.releng.git/commit/?id=929ddf6ae124834193e70c4fa8b30a8ce6ca071e I couldn't find anything else to complain about, but thought I would suggest you *might* want to consider using "import package" instead of "require bundle" ... which think would alway be my advice for anyone doing small bits of "low level" function which in theory could be used "as is" in many releases ... even if those bundles end up being refactored, or something. But, I doubt much advantage to it in practice. And, I did want to say, I do (still) think of this as "experimental". I do not expect it to change ... just did not want to give the false impression "once it is in a build it is written in stone" (that's only for releases :) But, I expect it to stay, in some form, and hope the input (reports) you get from it you find worth while. Much thanks for the contribution ... now you can start the hard work of improving performance! :)
(In reply to David Williams from comment #38) Thanks a lot, David. Much appreciated.
(In reply to David Williams from comment #38) > I've applied the Gerrit patch (after rebasing). > Add this bundle did cause a "releng" failure. See bug 444737.
