Community
Participate
Working Groups
To be honest, new index is dead because no one invest time to fix remaining issues and to keep it up to date with recent JDT changes. We see a number of bugs from users who enabled this preference in the hope to get a "better" JDT experience. Exactly the opposite is what they get. Deadlocks, empty type hierarchies etc. As the bug title says, I propose to hide this preference from "average" users, or move or rename it, so that "average" users would not be able to turn new index on just by simple clicking. Another option is to rename "new" to "experimental" or "unsupported" and add a warning dialog after enabling this. Also we could add some JVM option to enable this configuration. I'm open for any other idea to keep our users away from using new index.
+1
I was about to suggest to simply remove the option from the UI, but then realized that in workspaces that already have the new index enabled, users wouldn't even be able to see this fact. => Not a good idea.
(In reply to Stephan Herrmann from comment #2) > I was about to suggest to simply remove the option from the UI, but then > realized that in workspaces that already have the new index enabled, users > wouldn't even be able to see this fact. => Not a good idea. Not so nice, but we could only show the UI for users that enabled it already. The drawback is that we would need another preference to remember whether it was enabled once. We can't remove it for users who already used it. Another more honest approach would be to announce that we plan get rid of the new index and will therefore remove the UI and force the preference to be false.
I vaguely seem to remember hearing somebody from Red Hat say that they might work on the new index (at EclipseCon) Maybe we should give them an opportunity to speak up before we declare the new index dead? On the short run, I think that simply adding something like (experimental) to the "Enable new Java index" setting is a good idea.
If the preference is forced to false, the allocated disk space should be released automatically.
(In reply to Till Brychcy from comment #4) > I vaguely seem to remember hearing somebody from Red Hat say that they might > work on the new index (at EclipseCon) > > Maybe we should give them an opportunity to speak up before we declare the > new index dead? Yes, I heard that there too, but nothing since then. I will ask Alex in our next PMC call. Good point.
New Gerrit change created: https://git.eclipse.org/r/137800
(In reply to Till Brychcy from comment #4) > On the short run, I think that simply adding something like (experimental) > to the "Enable new Java index" setting is a good idea. "Enable new Java index" -> "Enable new (experimental) Java index" (In reply to Eclipse Genie from comment #7) > New Gerrit change created: https://git.eclipse.org/r/137800 I propose to push this for RC2, or RC1 if there will be a rebuild.
Gerrit change https://git.eclipse.org/r/137800 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=d790d06c6ada169190bfc53e912027a77b059a5b
(In reply to Andrey Loskutov from comment #8) > (In reply to Till Brychcy from comment #4) > > On the short run, I think that simply adding something like (experimental) > > to the "Enable new Java index" setting is a good idea. > > "Enable new Java index" -> "Enable new (experimental) Java index" > > (In reply to Eclipse Genie from comment #7) > > New Gerrit change created: https://git.eclipse.org/r/137800 > > I propose to push this for RC2, or RC1 if there will be a rebuild. I've merged the new version: "Enable new Java index" -> "Enable new Java index (experimental)" Let's keep this bug open to decide during 4.12 where this is going.
(In reply to Dani Megert from comment #6) > (In reply to Till Brychcy from comment #4) > > I vaguely seem to remember hearing somebody from Red Hat say that they might > > work on the new index (at EclipseCon) > > > > Maybe we should give them an opportunity to speak up before we declare the > > new index dead? > Yes, I heard that there too, but nothing since then. I will ask Alex in our > next PMC call. Good point. In today's PMC meeting Alex said that Red Hat sees no value in the new indexer. Anyone wants to take this bug and work on the removal of the feature? I'm not talking about removing all the code, but just retire the feature for now.
(In reply to Dani Megert from comment #11) > In today's PMC meeting Alex said that Red Hat sees no value in the new > indexer. > > Anyone wants to take this bug and work on the removal of the feature? I'm > not talking about removing all the code, but just retire the feature for now. We (at Advantest) are in *theory* interested in a more scalable JDT index, but I do not see a chance to get resources working on this area for the near future (1-2 years from now). So I'm fine with removing this option from UI.
Do we want to remove the option from UI in M1? Do we want to switch the option automatically back to the old index, or how we proceed with workspaces having new index on? Emit a warning to the log?
(In reply to Andrey Loskutov from comment #13) > Do we want to remove the option from UI in M1? > > Do we want to switch the option automatically back to the old index, or how > we proceed with workspaces having new index on? Emit a warning to the log? We should go with the honest approach from comment 3. So, first step would be to announce it and after about 2 weeks, remove the preference and set it to use the old index.
(In reply to Dani Megert from comment #14) > (In reply to Andrey Loskutov from comment #13) > > Do we want to remove the option from UI in M1? > > > > Do we want to switch the option automatically back to the old index, or how > > we proceed with workspaces having new index on? Emit a warning to the log? > We should go with the honest approach from comment 3. So, first step would > be to announce it and after about 2 weeks, remove the preference and set it > to use the old index. Can you take care of that?
(In reply to Dani Megert from comment #15) > (In reply to Dani Megert from comment #14) > > (In reply to Andrey Loskutov from comment #13) > > > Do we want to remove the option from UI in M1? > > > > > > Do we want to switch the option automatically back to the old index, or how > > > we proceed with workspaces having new index on? Emit a warning to the log? > > We should go with the honest approach from comment 3. So, first step would > > be to announce it and after about 2 weeks, remove the preference and set it > > to use the old index. > Can you take care of that? OK.
(In reply to Till Brychcy from comment #4) > I vaguely seem to remember hearing somebody from Red Hat say that they might > work on the new index (at EclipseCon) I was looking into creating an index that would use Lucene instead of the custom database. However, my activities stalled. There is a WIP here: https://github.com/eclipseguru/eclipse.jdt.core/commit/c6b0fc401294a93fde2a618970c0d37627bfcede
(In reply to Gunnar Wagenknecht from comment #17) > I was looking into creating an index that would use Lucene instead of the > custom database. However, my activities stalled. > > There is a WIP here: > https://github.com/eclipseguru/eclipse.jdt.core/commit/c6b0fc401294a93fde2a618970c0d37627bfcede > I wasn't aware of this work. Interestingly I and Philippe were just discussing this last week. Do you recall how close you were to make it work? I can try and take it to completion this if you found it promising. And if I understand it correctly, this is not bound to the new indexer and just changes storing and retrieving part to the disk, right?
(In reply to Jay Arthanareeswaran from comment #18) > Do you recall how close you were to make it work? I can try and take it to > completion this if you found it promising. There were quite a few open ends. > And if I understand it correctly, this is not bound to the new indexer and > just changes storing and retrieving part to the disk, right? No. This was based on the new indexer. I still believe that - to explore the full benefits - changing the storage format of the old indexer won't work. The major design issue of the new indexer is (IMO) implementing its own graph database. JDT should become a database vendor. When I talked to people at EclipseCon Europe my proposal was to leave the new indexer in but refactor its implementation into an extension. I would then provide an extension that is using Solr. FWIW, I wasn't complete on Lucene - I actually started with Solr because it already provides some functionality that the indexer needs. I also see a lot potential for the future with it. The extensibility requirement seemed important because people wanted JDT to be useful without having a strong dependency on Lucene/Solr.
New Gerrit change created: https://git.eclipse.org/r/141474
New Gerrit change created: https://git.eclipse.org/r/141475
(In reply to Eclipse Genie from comment #20) > New Gerrit change created: https://git.eclipse.org/r/141474 (In reply to Eclipse Genie from comment #21) > New Gerrit change created: https://git.eclipse.org/r/141475 Dani, those patches above remove preference from UI and disable the new index completely in JDT core. After that, only old index will be used. Do we want to merge those patches for M2?
(In reply to Andrey Loskutov from comment #22) > Dani, those patches above remove preference from UI and disable the new > index completely in JDT core. After that, only old index will be used. Do we > want to merge those patches for M2? Have you made sure the disk space is released as suggested in comment 5?
(In reply to Till Brychcy from comment #23) > (In reply to Andrey Loskutov from comment #22) > > Dani, those patches above remove preference from UI and disable the new > > index completely in JDT core. After that, only old index will be used. Do we > > want to merge those patches for M2? > > Have you made sure the disk space is released as suggested in comment 5? No. The problem here is that we will be forced to run preferences check and "new index init work" forever only for few cases where the new index was enabled. I don't think this is appropriate. If the disk space is a problem, deleting the workspace will be a solution too.
(In reply to Andrey Loskutov from comment #24) > (In reply to Till Brychcy from comment #23) > > (In reply to Andrey Loskutov from comment #22) > > > Dani, those patches above remove preference from UI and disable the new > > > index completely in JDT core. After that, only old index will be used. Do we > > > want to merge those patches for M2? > > > > Have you made sure the disk space is released as suggested in comment 5? > > No. The problem here is that we will be forced to run preferences check and > "new index init work" forever only for few cases where the new index was > enabled. I don't think this is appropriate. If the disk space is a problem, > deleting the workspace will be a solution too. Not very user friendly. Just checking if a preference is on, shouldn't be expensive, so I wonder why you think this in not appropriate (and I don't think we'd have to do it forever) I remember that people were complaining about the disk usage caused by the new index, so I think as absolutely minimum requirement it should work that a possibly still existing new index is cleared if a manual "Rebuild index" is done. Is that the case?
(In reply to Till Brychcy from comment #25) > I remember that people were complaining about the disk usage caused by the > new index [...] That was bug 515268
(In reply to Till Brychcy from comment #25) > (In reply to Andrey Loskutov from comment #24) > > (In reply to Till Brychcy from comment #23) > > > (In reply to Andrey Loskutov from comment #22) > > > > Dani, those patches above remove preference from UI and disable the new > > > > index completely in JDT core. After that, only old index will be used. Do we > > > > want to merge those patches for M2? > > > > > > Have you made sure the disk space is released as suggested in comment 5? > > > > No. The problem here is that we will be forced to run preferences check and > > "new index init work" forever only for few cases where the new index was > > enabled. I don't think this is appropriate. If the disk space is a problem, > > deleting the workspace will be a solution too. > > Not very user friendly. Just checking if a preference is on, shouldn't be > expensive, so I wonder why you think this in not appropriate (and I don't > think we'd have to do it forever) The problem I see is that there is not just the preference check but also some of the new index code must be kept to make it happen and some new code must be added to run on startup too - too much effort / code for obsoleted and not widely used feature. > I remember that people were complaining about the disk usage caused by the > new index, so I think as absolutely minimum requirement it should work that > a possibly still existing new index is cleared if a manual "Rebuild index" > is done. Is that the case? It was not, but this is a good point. We can cleanup "new index" files on reset index command, I will update the patch.
(In reply to Till Brychcy from comment #26) > (In reply to Till Brychcy from comment #25) > > I remember that people were complaining about the disk usage caused by the > > new index [...] > > That was bug 515268 This is not about "usual" workspaces, it is server side thing, I don't think we should worry about that. "Normal" users simply delete everything and start from scratch if they aren't happy with Eclipse, and this will automatically solve the problem of obsoleted data.
(In reply to Andrey Loskutov from comment #22) > Dani, those patches above remove preference from UI and disable the new > index completely in JDT core. After that, only old index will be used. Do we > want to merge those patches for M2? No. Please merge once we contributed to M2 (expected tomorrow).
Gerrit change https://git.eclipse.org/r/141474 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=1aa8ca0cc66b6a9da48e736bb8cd152e8d55ed7a
Gerrit change https://git.eclipse.org/r/141475 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=5faa7c5f1d6ced2be62840ecfb39438169bffa16
Andrey, AFAIR the 'Rebuild Index' button was only added for the new indexer (see bug 515534). We might also remove that UI and related code.
(In reply to Dani Megert from comment #32) > Andrey, AFAIR the 'Rebuild Index' button was only added for the new indexer > (see bug 515534). We might also remove that UI and related code. See bug 515534 comment 4 - the button rebuilds both, old and new index, also you can try it yourself, the old index starts to run immediately after clicking on that bitton. So I think we should keep it.
(In reply to Andrey Loskutov from comment #33) > (In reply to Dani Megert from comment #32) > > Andrey, AFAIR the 'Rebuild Index' button was only added for the new indexer > > (see bug 515534). We might also remove that UI and related code. > > See bug 515534 comment 4 - the button rebuilds both, old and new index, also > you can try it yourself, the old index starts to run immediately after > clicking on that bitton. So I think we should keep it. OK. Yes, I know it does it for both. Did you make sure it's not also rebuilding the new one? That should not happen after disabling it.
(In reply to Dani Megert from comment #34) > OK. Yes, I know it does it for both. Did you make sure it's not also > rebuilding the new one? That should not happen after disabling it. Yes. See org.eclipse.jdt.internal.core.nd.indexer.Indexer.rebuildIndex(IProgressMonitor) if (!JavaIndex.isEnabled()) { return; } rescan(subMonitor.split(97)); rescan() is only triggered if JavaIndex.isEnabled(), which is always "false" now.
The UI change is merged, the old index can't be used now. I will close this bug now. @All: I wonder what do we plan to do with the new index code, which is now unused anymore? Do we want to completely remove it? Even if unused it adds an extra complexity to the JDT core code, which is complex enough (if Index.isEnabled() ...). Also I assume we have tests for it which are still running. It is sad to remove it, but I don't see a value to keep dead code alive.
(In reply to Andrey Loskutov from comment #36) > The UI change is merged, the old index can't be used now. I will close this > bug now. I've reopened it to add an entry to the N&N. > @All: I wonder what do we plan to do with the new index code, which is now > unused anymore? Do we want to completely remove it? Even if unused it adds > an extra complexity to the JDT core code, which is complex enough (if > Index.isEnabled() ...). Also I assume we have tests for it which are still > running. > > It is sad to remove it, but I don't see a value to keep dead code alive. For now we should leave it in, at least for a release, and see whether there's some response from the community.
.
(In reply to Dani Megert from comment #37) > (In reply to Andrey Loskutov from comment #36) > > The UI change is merged, the old index can't be used now. I will close this > > bug now. > I've reopened it to add an entry to the N&N. Andrey, please add the N&N entry and close the bug.
(In reply to Dani Megert from comment #37) > I've reopened it to add an entry to the N&N. Please review: https://git.eclipse.org/r/#/c/142585/
Gerrit change https://git.eclipse.org/r/142585 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=5d269f1765f975d9849b82fc7f14efd64b3da096
(In reply to Dani Megert from comment #37) > > @All: I wonder what do we plan to do with the new index code, which is now > > unused anymore? Do we want to completely remove it? Even if unused it adds > > an extra complexity to the JDT core code, which is complex enough (if > > Index.isEnabled() ...). Also I assume we have tests for it which are still > > running. > > > > It is sad to remove it, but I don't see a value to keep dead code alive. > For now we should leave it in, at least for a release, and see whether > there's some response from the community. Now we are at 4.15, so far no one response from the community. The unused code adds a bunch of complexity to JDT codebase, and if no one uses that, we should remove it. It feels bad, but "dead" code is not nice either, and there are ~250 classes lying around, without tests.
