search

YunaBraska / github-workflow-plugin

Your Ultimate Wingman for GitHub Workflows and Actions! 🚀
https://github.com/YunaBraska/github-workflow-plugin
Apache License 2.0
41 stars 8 forks source link

UI freeze when opening a repo with large amount of JS files #39

Closed ForNeVeR closed 8 months ago

ForNeVeR commented 8 months ago

What happened? I've opened a project https://github.com/codingteam/emulsion in JetBrains Rider 2023.3 EAP4 and encountered a UI freeze caused by the plugin.

How can we reproduce the issue?

  1. Clone https://github.com/codingteam/emulsion
  2. I assume you'll have to either build the project or call npm install at least, to get node_modules with huge number of dependencies (which is normal for a frontend project).
  3. Open it in Rider
  4. See the UI freeze on startup

It became fine after several minutes.

Relevant log output I was able to call jstack on a stuck Rider instance to see this:

"AWT-EventQueue-0" #103 prio=6 os_prio=0 cpu=30906.25ms elapsed=167.28s tid=0x0000026b99264610 nid=0x2edc runnable  [0x000000917dffb000]
   java.lang.Thread.State: RUNNABLE
    at com.intellij.psi.impl.source.tree.CompositeElement.countChildren(CompositeElement.java:427)
    at com.intellij.psi.impl.source.tree.CompositeElement.getChildrenAsPsiElements(CompositeElement.java:390)
    at com.intellij.psi.impl.source.tree.CompositePsiElement.getChildren(CompositePsiElement.java:45)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:290)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:291)
    at com.github.yunabraska.githubworkflow.helper.PsiElementHelper.getAllElements(PsiElementHelper.java:282)
    at com.github.yunabraska.githubworkflow.helper.PsiElementChangeListener.lambda$childrenChanged$3(PsiElementChangeListener.java:33)
    at com.github.yunabraska.githubworkflow.helper.PsiElementChangeListener$$Lambda$6511/0x0000000802644f50.apply(Unknown Source)
    at java.util.Optional.map(java.base@17.0.8.1/Optional.java:260)
    at com.github.yunabraska.githubworkflow.helper.PsiElementChangeListener.childrenChanged(PsiElementChangeListener.java:33)
    at com.intellij.psi.impl.PsiManagerImpl.notifyPsiTreeChangeListener(PsiManagerImpl.java:414)
    at com.intellij.psi.impl.PsiManagerImpl.fireEvent(PsiManagerImpl.java:357)
    at com.intellij.psi.impl.PsiManagerImpl.childrenChanged(PsiManagerImpl.java:306)
    at com.intellij.psi.impl.file.impl.FileManagerImpl.forceReload(FileManagerImpl.java:140)
    at com.intellij.psi.impl.file.impl.PsiVFSListener.propertyChanged(PsiVFSListener.java:345)
    at com.intellij.psi.impl.file.impl.PsiVFSListener.fireForGrouped(PsiVFSListener.java:783)
    at com.intellij.psi.impl.file.impl.PsiVFSListener$$Lambda$4327/0x0000000801edc508.accept(Unknown Source)
    at one.util.streamex.CollapseSpliterator.lambda$forEachRemaining$0(CollapseSpliterator.java:133)
    at one.util.streamex.CollapseSpliterator$$Lambda$4328/0x0000000801edc760.accept(Unknown Source)
    at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(java.base@17.0.8.1/ArrayList.java:1625)
    at one.util.streamex.CollapseSpliterator.forEachRemaining(CollapseSpliterator.java:129)
    at one.util.streamex.AbstractStreamEx.forEach(AbstractStreamEx.java:352)
    at com.intellij.psi.impl.file.impl.PsiVFSListener.groupAndFire(PsiVFSListener.java:757)
    at com.intellij.psi.impl.file.impl.PsiVFSListener.after(PsiVFSListener.java:747)
    at com.intellij.psi.impl.file.impl.PsiVFSListener$1.after(PsiVFSListener.java:142)
    at java.lang.invoke.LambdaForm$DMH/0x0000000800800000.invokeInterface(java.base@17.0.8.1/LambdaForm$DMH)
    at java.lang.invoke.LambdaForm$MH/0x0000000800a8c800.invoke(java.base@17.0.8.1/LambdaForm$MH)
    at java.lang.invoke.LambdaForm$MH/0x00000008007ea400.invokeExact_MT(java.base@17.0.8.1/LambdaForm$MH)
    at com.intellij.util.messages.impl.MessageBusImplKt.invokeMethod(MessageBusImpl.kt:699)
    at com.intellij.util.messages.impl.MessageBusImplKt.invokeListener(MessageBusImpl.kt:663)
    at com.intellij.util.messages.impl.MessageBusImplKt.executeOrAddToQueue(MessageBusImpl.kt:491)
    at com.intellij.util.messages.impl.ToDirectChildrenMessagePublisher.publish$intellij_platform_core(CompositeMessageBus.kt:279)
    at com.intellij.util.messages.impl.MessagePublisher.invoke(MessageBusImpl.kt:448)
    at jdk.proxy2.$Proxy155.after(jdk.proxy2/Unknown Source)
    at com.intellij.util.FileContentUtilCore.lambda$reparseFiles$0(FileContentUtilCore.java:47)
    at com.intellij.util.FileContentUtilCore$$Lambda$6503/0x000000080297d7b8.run(Unknown Source)
    at com.intellij.openapi.application.impl.ApplicationImpl.runWriteAction(ApplicationImpl.java:989)
    at com.intellij.util.FileContentUtilCore.reparseFiles(FileContentUtilCore.java:35)
    at com.intellij.util.FileContentUtil.reparseFiles(FileContentUtil.java:21)
    at com.intellij.database.psi.DbPsiFacadeImpl.reparseDependentFiles(DbPsiFacadeImpl.java:154)
    at com.intellij.database.psi.DbPsiFacadeImpl.onLanguagesChanged(DbPsiFacadeImpl.java:130)
    at com.intellij.database.psi.DbPsiFacadeImpl.clearCachesImpl(DbPsiFacadeImpl.java:100)
    at com.intellij.database.psi.DbPsiFacadeImpl$1.lambda$clearCaches$0(DbPsiFacadeImpl.java:63)
    at com.intellij.database.psi.DbPsiFacadeImpl$1$$Lambda$5822/0x000000080281f708.run(Unknown Source)
    at com.intellij.util.concurrency.ContextRunnable.run(ContextRunnable.java:27)
    at com.intellij.openapi.application.TransactionGuardImpl.runWithWritingAllowed(TransactionGuardImpl.java:209)
    at com.intellij.openapi.application.TransactionGuardImpl.access$100(TransactionGuardImpl.java:22)
    at com.intellij.openapi.application.TransactionGuardImpl$1.run(TransactionGuardImpl.java:191)
    at com.intellij.openapi.application.impl.ApplicationImpl.runIntendedWriteActionOnCurrentThread(ApplicationImpl.java:857)
    at com.intellij.openapi.application.impl.ApplicationImpl$4.run(ApplicationImpl.java:477)
    at com.intellij.openapi.application.impl.RwLockHolder.runWithEnabledImplicitRead(RwLockHolder.kt:75)
    at com.intellij.openapi.application.impl.RwLockHolder.runWithImplicitRead(RwLockHolder.kt:67)
    at com.intellij.openapi.application.impl.ApplicationImpl.runWithImplicitRead(ApplicationImpl.java:1444)
    at com.intellij.openapi.application.impl.FlushQueue.doRun(FlushQueue.java:82)
    at com.intellij.openapi.application.impl.FlushQueue.runNextEvent(FlushQueue.java:124)
    at com.intellij.openapi.application.impl.FlushQueue.flushNow(FlushQueue.java:44)
    at com.intellij.openapi.application.impl.FlushQueue$$Lambda$436/0x00000008005197b0.run(Unknown Source)
    at java.awt.event.InvocationEvent.dispatch(java.desktop/InvocationEvent.java:318)
    at java.awt.EventQueue.dispatchEventImpl(java.desktop/EventQueue.java:792)
    at java.awt.EventQueue$3.run(java.desktop/EventQueue.java:739)
    at java.awt.EventQueue$3.run(java.desktop/EventQueue.java:733)
    at java.security.AccessController.executePrivileged(java.base@17.0.8.1/AccessController.java:776)
    at java.security.AccessController.doPrivileged(java.base@17.0.8.1/AccessController.java:399)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(java.base@17.0.8.1/ProtectionDomain.java:86)
    at java.awt.EventQueue.dispatchEvent(java.desktop/EventQueue.java:761)
    at com.intellij.ide.IdeEventQueue.defaultDispatchEvent(IdeEventQueue.kt:695)
    at com.intellij.ide.IdeEventQueue._dispatchEvent$lambda$12(IdeEventQueue.kt:589)
    at com.intellij.ide.IdeEventQueue$$Lambda$879/0x0000000800b97348.run(Unknown Source)
    at com.intellij.openapi.application.impl.RwLockHolder.runWithoutImplicitRead(RwLockHolder.kt:44)
    at com.intellij.ide.IdeEventQueue._dispatchEvent(IdeEventQueue.kt:589)
    at com.intellij.ide.IdeEventQueue.access$_dispatchEvent(IdeEventQueue.kt:72)
    at com.intellij.ide.IdeEventQueue$dispatchEvent$processEventRunnable$1$1$1.compute(IdeEventQueue.kt:355)
    at com.intellij.ide.IdeEventQueue$dispatchEvent$processEventRunnable$1$1$1.compute(IdeEventQueue.kt:354)
    at com.intellij.openapi.progress.impl.CoreProgressManager.computePrioritized(CoreProgressManager.java:793)
    at com.intellij.ide.IdeEventQueue$dispatchEvent$processEventRunnable$1$1.invoke(IdeEventQueue.kt:354)
    at com.intellij.ide.IdeEventQueue$dispatchEvent$processEventRunnable$1$1.invoke(IdeEventQueue.kt:349)
    at com.intellij.ide.IdeEventQueueKt.performActivity$lambda$1(IdeEventQueue.kt:1014)
    at com.intellij.ide.IdeEventQueueKt$$Lambda$878/0x0000000800b94710.run(Unknown Source)
    at com.intellij.openapi.application.TransactionGuardImpl.performActivity(TransactionGuardImpl.java:106)
    at com.intellij.ide.IdeEventQueueKt.performActivity(IdeEventQueue.kt:1014)
    at com.intellij.ide.IdeEventQueue.dispatchEvent$lambda$7(IdeEventQueue.kt:349)
    at com.intellij.ide.IdeEventQueue$$Lambda$875/0x0000000800b8ca00.run(Unknown Source)
    at com.intellij.openapi.application.impl.ApplicationImpl.runIntendedWriteActionOnCurrentThread(ApplicationImpl.java:862)
    at com.intellij.ide.IdeEventQueue.dispatchEvent(IdeEventQueue.kt:391)
    at java.awt.EventDispatchThread.pumpOneEventForFilters(java.desktop/EventDispatchThread.java:207)
    at java.awt.EventDispatchThread.pumpEventsForFilter(java.desktop/EventDispatchThread.java:128)
    at java.awt.EventDispatchThread.pumpEventsForHierarchy(java.desktop/EventDispatchThread.java:117)
    at java.awt.EventDispatchThread.pumpEvents(java.desktop/EventDispatchThread.java:113)
    at java.awt.EventDispatchThread.pumpEvents(java.desktop/EventDispatchThread.java:105)
    at java.awt.EventDispatchThread.run(java.desktop/EventDispatchThread.java:92)

Operating System Windows 11

Plugin Version 3.1.0

IDE Name and Version JetBrains Rider 2023.3 EAP4

Expected behavior The plugin should not perform file operations on the event dispatch thread, and should fall back to the background thread for this operation. Perhaps, an interruptible background read action would be more appropriate in this particular case.

Screenshots Nope.

Additional context Sadly, I'm not an expert on PSI structure in JetBrains IDEs (even though I am knowledgeable at some areas of IntelliJ plugin development), so I cannot tell from the top of my head what's the 100% correct approach to this. But perhaps I can help after studying more code of this plugin.

YunaBraska commented 8 months ago

Hello again @ForNeVeR 👋 It's strange that the Plugin freezes the UI for several minutes with only small GitHub Workflow files 🤔 💡 Maybe it has something to do with #38 There are many JetBrains operation which I assumed to be async in the background, and now I have to find and fix them all 😟 This is what I hope fixes it: ProjectStartup.java:92 (coming soon)

Speaking about code, I am refactoring again 🙈. Let me know if you have ideas and wishes to structure code. My current Idea is to put different logics from, e.g. steps to its own classes. Like here: Logic Package. Its not easy to structure plugin code 😓

ForNeVeR commented 8 months ago

JFYI, my current advice would be to just process the PSI changes asynchronously. Schedule them to a single-thread executor (not necessarily a single-thread one, but that would help if you care about the event ordering), or something like that, and it should work well. The side effect produced by the PsiElementChangeListener is already asynchronous, so it should be fine.

The only problem is that it may be a bit tricky to work with PSI from a background thread. But generally, you can start a read action from a background thread. If a particular PSI element is "valid", it means it will stay stably valid during the whole duration of the background read action. And if it isn't, then you should receive an updated version of it in your change handler eventually.

YunaBraska commented 8 months ago

Yes, that's a nice idea which I also had in the past. I was using then the PsiElement cache to tell the Annotator and ReferenceContributor what to do. The issue was, that the Annotator and ReferenceContributor got different PsiElement from what I parsed. Like one gets a YamlKeyValue, the other one gets a LeafElements and the next one a ScalarElement. Additionally, the PsiElement cache or backgroundtask was always out of sync, which made annotations coming up like 10 times as some PsiElements weren't parsed but still there. I really want to have a background task because I need to parse and implement the same logic everywhere in Annotator, ReferencContributor and more, a real nightmare. So I will try again, but not in this issue...

mauritz-lovgren commented 8 months ago

Same experience here. Works well on a small project with only workflow yml files, but freezes IntelliJ when opening large Vue project. I waited 10 minutes before I gave up and hard-removed the plugin to get IntelliJ working again. Would really like to see this fixed, as the plugin is an absolute must for detecting problems in GA workflow files!

YunaBraska commented 8 months ago

Version 3.2.0 is on the way. I wrapped the startup into its own thread. This should unblock the UI. Unfortunately, I could not reproduce the UI freeze. So please check the new version if the new Version at least unblocks the UI @mauritz-lovgren & @ForNeVeR .

It feels very strange to have a 10-minute freeze while I have timeouts, caches, .... I get the feeling that there is somewhere a side effect... In short: I will still investigate and work on it.

mauritz-lovgren commented 8 months ago

Version 3.2.0 is on the way. I wrapped the startup into its own thread. This should unblock the UI. Unfortunately, I could not reproduce the UI freeze. So please check the new version if the new Version at least unblocks the UI @mauritz-lovgren & @ForNeVeR .

It feels very strange to have a 10-minute freeze while I have timeouts, caches, .... I get the feeling that there is somewhere a side effect... In short: I will still investigate and work on it.

Sorry, still same behaviour with 3.2.0. IntelliJ freezes forever, large Vue project won't open. :-(

ForNeVeR commented 8 months ago

@mauritz-lovgren, could you check the freeze dumps? Are they the same? Or maybe attach the new dump here, it would help.

mauritz-lovgren commented 8 months ago

@mauritz-lovgren, could you check the freeze dumps? Are they the same? Or maybe attach the new dump here, it would help.

Sure! Attached the full jstack dump for when I open just my (somewhat large) Vue project (in IntelliJ IDEA 2023.2.4). I see yunabraska classes in action in several threads: yunabraska-jstack.txt

IntelliJ consumes a lot of CPU resources (stuck on about 200% on my Mac, two threads competing in a loop?), memory is stable. No IDE menus / UI elements are responding. I have not had the patience to wait long enough (hard killed IDEA after 10 minutes) to see whether the project actually opens in the end.

ForNeVeR commented 8 months ago

Ok, can confirm the core reason is the same. PsiManagerImpl.notifyPsiTreeChangeListener gets delivered on the event dispatch thread, and PsiElementHelper.getAllElements that's called in PsiElementChangeListener may take a long time apparently.

ForNeVeR commented 8 months ago

It is obvious it tries to parse JS files, which should not be the case, since the plugin is only interested in YML files?

YunaBraska commented 8 months ago

Yes, I am only interested in YAML files and also only in specific ones.

YunaBraska commented 8 months ago

@mauritz-lovgren Nice thread dump! I think I have missed adding one more filter! childrenChanged

YunaBraska commented 8 months ago

Version 3.2.1 is on the way. (waiting for approval) Hopefully that magic number fixes the issue. The more I write asynchronously, the less the tests work - now all my tests are failing, so I guess it's a good sign that something is now really async. I wonder how to Fix the tests now 🗡️

ForNeVeR commented 8 months ago

There are several general strategies on what to do in an IDE plugin test when the tested functionality is highly async. Pick any.

  1. Make the corresponding functionality to run synchronously if executed under tests.
  2. Get a separate executor service / pool for the functionality, and make the test to wait for the pool to become empty/free before proceeding.
  3. Make the tests to wait for the test condition for a while. For example, instead of Assert.equal("testResult", document.someProperty()) write a loop that waits for the condition (and pumps the event dispatch thread) to become true with a small timeout.
mauritz-lovgren commented 8 months ago

There are several general strategies on what to do in an IDE plugin test when the tested functionality is highly async. Pick any.

  1. Make the corresponding functionality to run synchronously if executed under tests.
  2. Get a separate executor service / pool for the functionality, and make the test to wait for the pool to become empty/free before proceeding.
  3. Make the tests to wait for the test condition for a while. For example, instead of Assert.equal("testResult", document.someProperty()) write a loop that waits for the condition (and pumps the event dispatch thread) to become true with a small timeout.

Regarding this specific issue: what about a unit test that verified that the stream filter + map function in the helper class actually picked the correct files as a start? That might have prevented the ill-behaviour without the need for advanced async testing?

Should you need async testing still, I can recommend Awaitility which relieves some of the burdens of writing tests for multi-threaded / async code.

mauritz-lovgren commented 8 months ago

Working well with version 3.2.1. Thank you for the fix!

YunaBraska commented 8 months ago

Finally :D, Thanks for the feedback 👍