Closed cdietrich closed 1 year ago
TODO:
can be reproduced locally.
have the feeling closing the editors does not longer work
no the document seems to leak somewhere else
@szarnekow do you have any platform change in mind that could cause this?
Since which IBuild is it? There were few PR's for platform text merged recently.
I haven't checked what is inside, but they seem to be big enough to introduce a leak somewhere.
the first time i observed it is today. need to check older logs
Regarding JDT: jdt.core tests run with 1 GB heap, and that was a good memory leak detector in the past, and there were no significant changes in JDT UI AFAIK
ok a lot of backtraces looks like
so there seems to be a job with a link to the reconciler
of course we dont see a previous run on jenkins cause the history was pruned
Looks like the jobmanager keeps a reference around?
to me it looks like the pending jobs dont really reduce. but i also dont see a difference to older versions. i wonder if we need to call something in the test to let the notifyListeners run. maybe there is also something that takes the listeners to run longer, so that the trottler actually throttles
adding an additional read and dispatch loop to setup the the tests seems to help. but what has changed ...
and of course i cannot oomph against older target :( https://github.com/eclipse/xtext-eclipse/issues/1960
now it gets funny. i see the job list also grow in 2022-03. will take a snapshot there too. there seem to be no reconciler leaks ... :(
there seem to be no job infos with reconciler in older versions. need to debug who creates them on latest.
stacktrace where the job infos are created
JobInfo.<init>(Job) line: 57
ProgressManager.lambda$14(Job) line: 613
0x0000000800f77580.apply(Object) line: not available
HashMap<K,V>.computeIfAbsent(K, Function<? super K,? extends V>) line: 1220
ProgressManager.progressFor(Job) line: 613
ProgressManager$1.updateFor(IJobChangeEvent) line: 502
ProgressManager$1.scheduled(IJobChangeEvent) line: 474
0x0000000800db9138.notify(IJobChangeListener, IJobChangeEvent) line: not available
JobListeners.sendEvent(JobChangeEvent) line: 131
JobListeners.sendEventsAsync(InternalJob) line: 118
JobListeners.waitAndSendEvents(InternalJob, boolean) line: 78
JobManager.withWriteLock(J, Function<J,T>) line: 524
JobManager.schedule(InternalJob, long) line: 1373
XtextReconciler(InternalJob).schedule(long) line: 396
XtextReconciler(Job).schedule(long) line: 684
XtextReconciler.resume() line: 345
XtextReconciler$DocumentListener.assistSessionEnded(ContentAssistEvent) line: 180
ContentAssistant.lambda$12(IContentAssistProcessor) line: 2521
0x000000080188dd20.accept(Object) line: not available
Collections$SingletonSet<E>.forEach(Consumer<? super E>) line: 4905
ContentAssistant.fireSessionEndEvent() line: 2518
CompletionProposalPopup.hide() line: 1110
ContentAssistant.hide() line: 2267
ContentAssistant.uninstall() line: 1609
XtextSourceViewer(SourceViewer).uninstallTextViewer() line: 1346
XtextSourceViewer(SourceViewer).unconfigure() line: 729
XtextSourceViewer(SourceViewer).handleDispose() line: 789
XtextSourceViewer(ProjectionViewer).handleDispose() line: 1303
XtextSourceViewer(TextViewer).lambda$1(DisposeEvent) line: 1741
on older versions 1st: org.eclipse.jface.text.contentassist.ContentAssistant.removeCompletionListener(ICompletionListener) 2nd : org.eclipse.jface.text.contentassist.ContentAssistant.fireSessionEndEvent()
on latest the remove seems not come before the fire
At least in SWTBot tests, it helps (at least it reduces flakyness) to wait for jobs to finish, by looking at boolean idle = Job.getJobManager().isIdle();
. SWTBot tests don't run in the UI tests, while these tests do, so the wait should be adapted.
to me it looks like the order of swt events has changed i have zero clue why.
bingo: the old code removes the reconciler first, content assistant 2nd the new does remove the content assistant first.
@iloveeclipse @angelozerr is this an intentional change with https://github.com/eclipse-platform/eclipse.platform.text/commit/81a446557dd3f8f354a272de447fe787881dbe1f
@iloveeclipse @angelozerr should this be a linked hashset in platform instead?
@iloveeclipse @angelozerr is this an intentional change with https://github.com/eclipse-platform/eclipse.platform.text/commit/81a446557dd3f8f354a272de447fe787881dbe1f
no, we wanted that all feature like reconciler, content assist use the same pattern lifecycle.
@iloveeclipse @angelozerr should this be a linked hashset in platform instead?
I think it can be a good idea, is it working better with linked hash map?
i will have a look later. we might then also have to iterate reverse
@szarnekow can you also check if there is something bogus with our reaction to the content assistant uninstall? or is the content assistant also created while the editor stays open?
I think it can be a good idea, is it working better with linked hash map?
I'd rather use a list and walk it backwards during uninstall.
I'd rather use a list and walk it backwards during uninstall.
I created a PR https://github.com/eclipse-platform/eclipse.platform.text/pull/121
pr looks good
Jenkins OOMS for Java 17 + Eclipse 4.27 https://ci.eclipse.org/xtext/job/xtext-eclipse/job/master/3214/console
.log
file is here https://ci.eclipse.org/xtext/job/xtext-eclipse/job/master/3214/artifact/org.eclipse.xtext.xbase.ui.tests/target/work/data/.metadata/.log