Community
Participate
Working Groups
Build Identifier: 0.7.0.v201110080901 With today's build, I found a big performance issue after using the VE for some time (Jiyong also mentioned that the VE has big performance issue after using it for some time). But we could not find a very clear scenario to reproduce the problem - lots of operations have been made before the problem happen, so it's not easy to know the exact step to reproduce the problem. This problem will make user very very unhappy - the VE always in refresh status and it will never be ended. The problem cannot be even removed after cleaning the workspace! I spent some time and reproduced the problem this afternoon, but not sure if this is the same problem we encountered during our normal usage of EDT. 1) In a new workspace, create an EGL RUI project and only import org.eclipse.edt.rui.widgets_0.7.0. 2) Right click the client package, and create an RUI handler named H1. 3) EDT open the H1 with VE, and drag a button to the grid layout. 4) Save the H1. 5) Click the Preview tab, and click the 'Show the web page in an external web browser' button. Keep the handler open in the browser, and switch window back to the EDT. 6) Close the H1. 7) Double click the H1 in the project explorer, and open the file with VE. 8) Repeat the step 5 to 7. In a certain loop, the 7th step cannot be finished, the VE cannot open the H1. In my testing, normally the problem will happen after repeat 4 times, and in some cases, more times are required. After the problem happened, clean the workspace also does not work. I set the defect severity to critical, let's discuss if you don't agree with it. Reproducible: Always
I printed the time spent on each step, HTMLGenerator is taking most of the time. It takes 3-20 seconds to generate HTML in my thinkpad. Generate dependent CSS seemed to be time consuming.
A simpler way to reproduce the problem 0. You must use SUN JDK for EDT The other steps are the same as Rocky said except: In step 5, you don't have to open in external browser I find that after open and close the RUI Handler several time, the evserver gets very slow. So that the VE seems dead. The problem is in line 1408 of EvServer.java final Socket client = serverSocket.accept(); This line consumes a lot of time.
Firstly, as Jiyong said, the problem only existed in the SUN JDK, IBM JDK does not have the problem. I dumped the threads when VE in 'dead', you can refer to the attached document for more information. From the thread dump, I noticed that below thread is in waiting status. "Visual Editor Event Queue Processor" prio=6 tid=0x00f30000 nid=0x1c54 waiting on condition [0x0aebf000] java.lang.Thread.State: TIMED_WAITING (sleeping) at java.lang.Thread.sleep(Native Method) at org.eclipse.edt.ide.rui.server.EvServer$QueueProcessor.run(EvServer.java:611) Locked ownable synchronizers: - None While the thread Jiyong mentioned is in running state, "Thread-6" prio=6 tid=0x041de400 nid=0x19a4 runnable [0x0980f000] java.lang.Thread.State: RUNNABLE at java.net.PlainSocketImpl.socketAccept(Native Method) at java.net.PlainSocketImpl.accept(PlainSocketImpl.java:408) - locked <0x1d5b8808> (a java.net.SocksSocketImpl) at java.net.ServerSocket.implAccept(ServerSocket.java:462) at java.net.ServerSocket.accept(ServerSocket.java:430) at org.eclipse.edt.ide.rui.server.EvServer.startServer(EvServer.java:1408) at org.eclipse.edt.ide.rui.server.EvServer$1.run(EvServer.java:686) I read the code of line 611, this line will be executed only when event queue is empty. I'm not sure if this is the root cause of the problem - should the event queue empty here? If it should not be empty, do we forgot to add the event to queue in certain conditions, or remove the event unexpectedly?
Created attachment 204856 [details] The thread dump file.
The problem is caused by the EvServer did not get the request from browser, seem's that the status of the browser is in a mess, and it did not send request to EvServer when the problem happened. It gets OK after restart the EDT.
Let's keep this bug for the VE 'dead' issue. I'll open another bug for HTMLGen performance.
*** Bug 360383 has been marked as a duplicate of this bug. ***
Open a handler in external browser, then close VE. This will cause the browser sending a lot of __getevent request to EVServer. EVServer will return 404 to __getevent if the contextKey is not found, and when browser get 404, it will send another __getevent immediately (actually wait for 10ms ). I made a fix so that EVServer will return "context terminate" together with 404, and have browser side check for "context terminate" and stop sending more request. While this probably is not the root cause of this problem, the fix did make performance better in my machine, and it is more difficult to run into the scenario described in this bug.
In recent 2 days, people are not seeing this problem any more. And Rocky is not able to reproduce it with the steps in the description. I think both the fix in comments 8 and the generator performance enhancement in bug 357432 has positive effect. The problem is either fixed indirectly or become very difficult to reproduce. I'll close the bug.
Today, testers found some performance problems with the VE. So reopen the defect.
I think it would be good if we start getting very specific with the configurations that people are using when having problems, including: * JRE (Provider and Version) * Eclipse Version * VE Browser * Description of the scenario leading up to the poor performance (if possible) (i.e. Drag and Drop a lot of widgets from the palette, lots of open editors, etc).
(In reply to comment #11) > I think it would be good if we start getting very specific with the > configurations that people are using when having problems, including: > > * JRE (Provider and Version) > * Eclipse Version > * VE Browser > * Description of the scenario leading up to the poor performance (if possible) > (i.e. Drag and Drop a lot of widgets from the palette, lots of open editors, > etc). Thomas found a way to reproduce the problem, below is his steps. 1. DD a Dojo BorderContainer into a handler. 2. DD another BorderContainer into the newly BorderContainer in step1. 3. DD some widgets into the 2nd level BorderContainer. 4. Go to preview tab to show, the tab will be hung for about 2 or 3 mins.
How does the scenario in comment 12 work in RBD?
(In reply to comment #13) > How does the scenario in comment 12 work in RBD? It works fine in my testing with RBD 8012.
Created attachment 206569 [details] cache
I have made a patch to cache RUI JS Runtime files, it can improve the performance of EvServer, but the root cause is at JS Runtime and widgets. Thomas' test case can work well with WebKit, but slow with IE. So I change the owner to Ji Yong.
Do we have some performance numbers that show how much improvement was gained by this fix?
In the loadScript method of the EVServer, the code was changed to add all loaded files to the cache. Are these files ever removed or updated? What files are placed in the cache from this method?
Lower the severity to normal for following reason told by developers, - The WebKit has some of performance improvements comparing to the IE, and now it will automatically detect the WebKit is installed or not. Maybe we can post some tips in EDT forum to tell users if they have the VE performance issue. Also, the XULRunner is under CQ process, and it will gain some of performance improvement if set it as the default browser. We can release the developer to other more important works.
Created attachment 206655 [details] remove cache in loadScript
(In reply to comment #18) > In the loadScript method of the EVServer, the code was changed to add all > loaded files to the cache. Are these files ever removed or updated? What > files are placed in the cache from this method? The code is from RBD, I did a investigation, the loadScript is only called when first dnd a new widget onto handler, at this time, the widget js will add to the cache, but when dnd the widget onto handler again, the loadScript will not be called, so nothing will read from the cache. I think the cache in loadScript can be removed and attached another fix.
The fix is in now. Thanks.
(In reply to comment #17) > Do we have some performance numbers that show how much improvement was gained > by this fix? Without the cache, it will cost 5s to read the runtime files, with the cache, it will only use 2s. so it is about 150% improve.
FYI we are not able to ship XULRunner with EDT, so you can't count on that performance enhancement.
I found newly reproduce steps whatever browsers you select. 1. DD a HTML widget into a handler. 2. Select HTML and open its properties view. 3. Choose position as absolute, then set x as 50, y as 50. 4. Choose position property as sttic. 5. Choose position property as absolute again. 6. Do NOT save the handler during these steps, then go to preview tab directly. Actual results: The preview page is loading and displaying as a blank page for a long time. Please refer to screenshot(20111114.jpg). But the page will load successfully after 2 or 3 mins finally.
Created attachment 206908 [details] screen shot on 20111114
Forest, I think the cache is not handling below files correctly because there's "runtime" in the package name. Please review the code. RUI_RUNTIME_JAVASCRIPT_FILES.add("org/eclipse/edt/eunit/runtime/AssertionFailedException.js"); //$NON-NLS-1$ RUI_RUNTIME_JAVASCRIPT_FILES.add("org/eclipse/edt/eunit/runtime/ConstantsLib.js"); //$NON-NLS-1$ RUI_RUNTIME_JAVASCRIPT_FILES.add("org/eclipse/edt/eunit/runtime/Log.js"); //$NON-NLS-1$ RUI_RUNTIME_JAVASCRIPT_FILES.add("org/eclipse/edt/eunit/runtime/LogResult.js"); //$NON-NLS-1$ RUI_RUNTIME_JAVASCRIPT_FILES.add("org/eclipse/edt/eunit/runtime/MultiStatus.js"); //$NON-NLS-1$ RUI_RUNTIME_JAVASCRIPT_FILES.add("org/eclipse/edt/eunit/runtime/Status.js"); //$NON-NLS-1$ RUI_RUNTIME_JAVASCRIPT_FILES.add("org/eclipse/edt/eunit/runtime/ServiceBindingType.js"); //$NON-NLS-1$ RUI_RUNTIME_JAVASCRIPT_FILES.add("org/eclipse/edt/eunit/runtime/targetLangKind.js"); //$NON-NLS-1$ RUI_RUNTIME_JAVASCRIPT_FILES.add("org/eclipse/edt/eunit/runtime/TestListMgr.js"); //$NON-NLS-1$
in 362539, we plan to combine all the runtime files to one file, I will modify the cache to fit the changes and retest to see if the performance problem also occurs.
The problem mentioned in comment 25 is because we are initializing a new handleIDEEvent loop every time a property value is changed in VE (or doing anything that cause a __reloadHandler). This was incorrect, and was cause by our changes in startupInit I've committed a fix which ensure there's only one handleIDEEvent loop.
When reviewing event processing in VE, I noticed that whenever a ReloadHandler happens (this could be caused by setting a property value or anything that changes the handler), we are actually reloading all the dependent js files which includes all the widgets. This is mostly not necessary. I've committed a fix which only reloads the handler js. This should improved the performance of most VE operations like change property, dnd a new widget etc.
Created attachment 207090 [details] fix for comment 29
Created attachment 207091 [details] fix for comment 30
I spend more time on event processing. The only VE event that is sent from EVServer to browser is "egl.evTerminateReloadHandler()" which is used whenever a change is made to the source code and request an incremental refresh of browser. This even could only happen in Design mode. The fact seemed to be 1. Preview mode does not accept event at all 2. When we need event to control preview mode behavior, we will have to redesign the EVServer.QueueProcessor. Currently, there could be multiple preview mode browsers connecting to one preview Context, and it is not possible to know which browser receives the event. 3. The event processing, even if not doing anything real, is causing performance issues. (This bug is partially related to the event in preview mode) Given that, I made a patch to disable event process for preview mode. I have tested the patch with RUISample, DojoSample, and all kinds of operation I can think of on VE.
Created attachment 207140 [details] disable event loop for preview mode
*** Bug 361773 has been marked as a duplicate of this bug. ***
Tony, after applying the changes in comment 34, what was your experience with using the IDE and external browsers? Did you notice any improvements? Was the improvement significant? I agree that we need to determine why the preview pane and external browsers are creating comet calls with the IDE, and this is a great find, but this code is the same in RBD, so I am wondering why it might be causing a problem now. Is the code the same as it was in RBD, or did something change in EDT? Regarding the comment about, "which external browser should receive a message from the Queue Processor", I believe the answer is "all of them".
Brian, you are right that something changed in EDT which makes this no-harm event loop impact performance now. In EDT, RUI loads each handler's js at runtime as opposite to RBD which put them all in one HTML. We are generating the handler's js to the context directory and EVServer has to be able to provide that js to browser. An URL to a js WITH context will be processed by the QueueProcessor of that context. If that QueueProcessor, at the same time, is also processing __getevents, it could delay the request to serve js file. In some situations, QueueProcessor will sleep for a second before it process the next event. And this gets worse if there are mutiple event loops (for example, you have both preview tab open and an external browser open for the same handler). I believe this is the root cause of this performance issue.
(In reply to comment #36) Regarding the performance improvement, I would say it's a big improvement. Previously, if you are opening Preview tab, and external browsers, working for a while, you'll start to see a delay of several seconds to more then 10 seconds to display a page. It gets worse when you open more VEs and more external browsers. Now I don't see that again even if I'm opening a lot of window. If you work on a simplest Handler with one VE open, it was fast even before this fix.
As Tony mentioned, a major difference between EDT and RBD is that we load ALL dependent JS files at runtime, instead of copying them into the HTML file. This requires us to load all files within the context of the browser (e.g. append a context key to the URL). This new context based processing, in addition to our existing comet calls with the server, can cause the EVServer's processing queues to become overloaded for each context. Due to the fact that we have not found a reason for the Preview and External Browsers (non-debug) to start comet calls with the IDE, turning off these comet calls would free up the processing queues to handle file load requests. We are going to disable comet calls for Preview and External Browsers (non-Debug) for .7.
Tony - I reverted the change to egl.canSendEventToIDE() where it was disabled for preview. This caused a pretty major problem where if you switch from preview to source, modify some code, and switch back to preview you get an "Internal generation error" in the browser. Looks like we found a place that was using this event after all.
Justin, Thanks for catching this. It turned out to be the way I disable event loop is more then just disable event loop, but also prevent browser to load anything from the IDE gateway. In your scenario, when user clicks preview after source change, the preview browser is requested to do a evTerminateReloadHandler which will need to load the updated handler from EVServer. However, this is not through event loop, but by a direct request to the Browser object to execute a piece of script. I changed the fix to limit it just to event loop. The egl.canSendEventToIDE function remains unchanged. The revised fix will be in egl.startHandleIDEEvent like below. egl.startHandleIDEEvent = function(){ if(egl.canSendEventToIDE() && (egl.enableEditing || egl.debugg)){ egl.handleIDEEvent(); } egl.startHandleIDEEvent = function(){}; };
That fix looks better, will let you know if I run into any other troubles.
The fix was verified in build v201111182101. If user opens many VE, there's still chances to hit the performance issue (slow painting of the view). I guess the event loop in design mode still has problem working together with the context based processing of js files. We should revisit the context based processing approach in 1.0. It is not necessary all dependent js files has to be context aware. Only the files in the working copy can possibly be generated to context directory, thus need to be processes by context. Other JS (widgets for example) can be processes by EVServer without knowning context. The other thing to be looked at might be the event loop in design mode, we can actually call the SWT Browser object to execute Script directly, maybe we can remove even loop for design mode as well. I'm going to lower the priority of the bug and set the version to 1.0 to address the problems described above.
*** Bug 364208 has been marked as a duplicate of this bug. ***
Will, to confirm that the remaining issues are definitely related to loading dependent files with a context key, please run the deployed version of your application in Chrome and see if the timings improve. Since we don't have context keys in the deployed version, the timings should improve if this is the problem. If the timings do not improve, then the issues is loading many files. If we can confirm that this issue does not occur in the deployed version of the application, I would like to propose the following change for development mode only; instead of loading each file with its own ajax call, we can send the entire list of required files to the EVServer at one time with a context key, and the server can respond with a single .js file that gets added to a script tag. This allows us to avoid the synchronous behavior of the QueueProcessor, and still avoid creating a large HTML file. As Tony mentioned, I agree the bottle neck in development mode is most likely due to the fact that we are loading all dependent files with a context key. NOTE: I believe the context key will always be required for these files, since a context key gives us the EGLPath to use when resolving the file. Without the key, we may resolve the wrong part if more than one part exists with the same name. In the EVServer, the QueueProcessor for each context key runs in a separate thread. When a request comes into the EVServer, it is put on the appropriate queue and the server goes to sleep. The QueueProcessor threads are designed to sleep for 10ms between requests, so the processor then wakes up and processes the request for the .js file. When loading all of the dependent files on RUI application startup, one .js file is loaded at a time, with the load of the next file only happening once the previous file is loaded and parsed by the browser. This means that the following is happening: * Browser requests a file and blocks * EVServer gets the request, puts it on the queue and sleeps * QueueProcessor wakes up and process the file and goes back to sleep * Browser receives the file, parses the contents, and requests the next file I am guessing that the constant thread swapping in the EVServer is the cause for our delay. If the problem happens in the deployed version of the application as well, the problem could be related to the synchronous nature of our load calls, and the fact that we are not creating link tags. I realize that this is required, and our .js files must be loaded in a specific order or issues will occur, but I am wondering if the link tags of the browser allow simultaneous file loads, where the contents are still parsed in the order they are received?
To be specific, I think we should leave egl.loadScript(s) alone for deployed applications, and add a call to "egl.loadIDEURL" for "_loadDependentFiles". The generated HTML file can use "_loadDependentFiles" for development and egl.loadScript(s) for deployment.
I verified the scenario in comments #25 on build of 20111120. The performance is good. Test team will try more scenario during regression tests.
We are going to stop working on this for 0.7.0, as further changes are too high risk. Deferring since we haven't achieved our final goals, so we'll keep working on it in the next release.
This falls under the JavaScript Runtime Performance theme for .8.
Disabled event loop for design mode, preview mode and external browser . Design mode will call Browser object to execute JS for page reloads. This is the only place that needs event loop previously. Debug still has its own event loop which was not touched.
I created a separate bug to track the contextKey fix, contextKey is only needs to load the handle being displayed in VE and any parts that exists in the same file as the handler. Bug 368001 - Only use contextKey to load parts in the file being displayed in VE (In reply to comment #37) > Brian, you are right that something changed in EDT which makes this no-harm > event loop impact performance now. In EDT, RUI loads each handler's js at > runtime as opposite to RBD which put them all in one HTML. We are generating > the handler's js to the context directory and EVServer has to be able to > provide that js to browser. An URL to a js WITH context will be processed by > the QueueProcessor of that context. If that QueueProcessor, at the same time, > is also processing __getevents, it could delay the request to serve js file. In > some situations, QueueProcessor will sleep for a second before it process the > next event. And this gets worse if there are mutiple event loops (for example, > you have both preview tab open and an external browser open for the same > handler). I believe this is the root cause of this performance issue.
I believe this bug is resolved with the various efforts around VE performance improvement. Close it.
No such performance problem is hit now, close the bug.
