Community
Participate
Working Groups
+++ This bug was initially created as a clone of Bug #117746 +++ Loading large files in an editor needs a lot of memory (up to 5 times the file size, see Bugzilla 75086), so the Workbench can run into an OutOfMemory error or freeze for some time while an editor is busy loading a file. Since users do not see in the Resource navigator how large a file is, it can easily happen that one tries to open a very large file by accident. We have auto-generated log files which range from 1 to 400 MB in size. The smaller ones we want to open in Eclipse Workbench, the larger ones we open in an external editor. Trying to opening a 400MB file in the Workbench by accident leads to an OutOfMemoryError and destabilizes the workbench or even makes it unusable for some time. It should be possible to define a maximum file size (in the Preferences, in plugin.xml or by computing available heap size) up to which an editor editor can open a file directly. If the user tries to open a file that exceeds the given size, the user should be shown a dialog which allows to * Open the file in spite of the warning * Open the file in an alternative (external) editor instead * Cancel the edit request Perhaps the best solution would be to extend the Editor API such that an editor is given the file path to be opened BEFORE it is actually opened. Thereby, each editor implementor could have policies for allowing or denying the open request. Policies for denying could be based on file size, virtual vs. real file system [See Bugzilla 106176], file content...) In case an edit request is denied, the Platform could show a dialog like "Editor FooBar has denied opening 'yourFile.xxx' with the following message: (Custom Message e.g. File is 50MB large) Do you want to * Open the file in (Dropdown of registered editors for xxx files) instead * Open the file in spite of the warning * Cancel "
Cloned for backport on R3_4_maintenance.
Created attachment 194199 [details] patch
Created attachment 194200 [details] patch (with copyright headers updated)
This patch adds a new (internal) preference that can be set in the plugin_customization.ini file as follows: org.eclipse.ui.workbench/LARGE_DOC_SIZE_FOR_EDITORS=20000000 Opening files that are larger than the threshold leads to a prompt that lets the user select an editor to be used for opening the file, with an opportunity to cancel. If the preference is not set (which is the default), the behaviour is unchanged. This change does not address the following cases: - Opening an editor on an external file using File>Open File... - Open editors that are re-opened in a new Eclipse session - Editors that operate on more than one file, such as the PDE editors - Opening files that are not accessible through a path on the local file system - Editors that are being reused by client code, for example when opening the compare editor repeatedly, or opening subsequent search results. This behaviour can be turned off in the preferences ("reuse open compare editors" on the Team preference page, "reuse editors to show matches" on the General>Search preference page) Since we are not able to predict the actual memory needs of any given editor, Eclipse may still run out of memory by opening several files that are under the threshold. I would recommend enabling the heap status from the General preference page to monitor actual memory consumption.
Oleg, would you be able to review this patch please?
Could you describe how this new option is supposed to be set? Few small things: - EditorManager#getAlternateEditor(): the Platform.getProduct() may return null - EditorManager#MAX_FILE_SIZE - could you change it to "long" instead of "int": a) The "int" size will break on files > 2Gb; b) value is compared against java.io.File.length() which uses "long" - EditorManager#isLargeDocument() I'd add a check for file.exists() before comparing file sizes, just to be sure Style: - EditorManager#CHECK_DOCUMENT_SIZE, #MAX_FILE_SIZE - usually this style of capitalization is used for constants. - Update copyright on EditorSelectionDialog - EditorManager formatting seems to be off in the added methods
(In reply to comment #6) > Could you describe how this new option is supposed to be set? Never mind, I just noticed that you have the description in the comment 4.
Created attachment 194501 [details] updated patch
(In reply to comment #6) > Few small things: > - EditorManager#getAlternateEditor(): the Platform.getProduct() may return null Good catch, this is now handled properly. > - EditorManager#MAX_FILE_SIZE - could you change it to "long" instead of "int": > a) The "int" size will break on files > 2Gb; > b) value is compared against java.io.File.length() which uses "long" done > - EditorManager#isLargeDocument() > I'd add a check for file.exists() before comparing file sizes, just to be > sure My gut feeling tells me that less calls into the java.io API is better (less chance of hitting obscure edge cases). The JavaDoc for File.length() says that it returns 0 if the file does not exist. > Style: > - EditorManager#CHECK_DOCUMENT_SIZE, #MAX_FILE_SIZE - usually this style of > capitalization is used for constants. done > - Update copyright on EditorSelectionDialog done > - EditorManager formatting seems to be off in the added methods done. Thanks for the review! Oleg, could you please review again? If you don't find anything else, would you be able to release this into R3_4_maintenance please? Thanks! Prakash, once we are happy with this for 3.4.2+, could you please get the change into R3_5_maintenance, R3_6_maintenance, and HEAD? Thanks!
I've (optimistically, assuming Oleg's review doesn't find anything) released the patch to R3_4_maintenance. I also updated the bundle version number for org.eclipse.ui.workbench to 3.4.3.qualifier.
(In reply to comment #9) +1, looks good.
The patch is problematic, since it also breaks users that want to open the file in the system editor or in an external editor they've configured on the "File Associations" preference page. At least for external editors, the dialog should not be shown and the opening should proceed normally. I would do the same for the system editor. And for the default editor, if it is an external editor. Smaller problems: - Javadoc of IPreferenceConstants.LARGE_DOC_SIZE_FOR_EDITORS should end with: * This preference is a <code>long</code> value that represents the * threshold in bytes. The default value is <code>0</code>. * * @since 3.4.3 */ - Add this to /org.eclipse.sdk/plugin_customization.ini ? # threshold to determine whether a document is large or not (in bytes) org.eclipse.ui.workbench/LARGE_DOC_SIZE_FOR_EDITORS = 0 - Embedding the product name in the new UI string "This may affect the stability of {0}." is not readily translatable. In some languages, the preposition needs a grammatical case, which varies with the product name. Even in English, "stability of Eclipse SDK" sounds strange (should rather be "stability of the Eclipse SDK"). I would remove the product name and just use "stability of the application".
Created attachment 194565 [details] updated patch++ Fixes the problems from comment 12.
.
Created attachment 194613 [details] updated patch++2 (In reply to comment #12) > The patch is problematic, since it also breaks users that want to open the file > in the system editor or in an external editor they've configured on the "File > Associations" preference page. Good catch! Thanks Markus. I am attaching a new patch based on yours, the only notable change was to remove the change to plugin_customization.ini. Products built on top of Eclipse will have their own branding plugin, so we need to be able to handle the preference not being set anyway. Also, on our side, we would have to make this change in org.eclipse.platform as well to be consistent.
(In reply to comment #15) > Created attachment 194613 [details] > updated patch++2 +1.
(In reply to comment #16) > (In reply to comment #15) > > Created attachment 194613 [details] [details] > > updated patch++2 > > +1. Released to R3_4_maintenance and updated the map file.
Created attachment 194721 [details] Screenshot on GTK I had a bad gut feeling about the change from Label to Text in the EditorSelectionDialog. When I tried it on GTK, I saw that the widget background is now white on that platform. What was the reason for not using "new Label(contents, SWT.WRAP)"? That looked good for me on Windows and GTK. If it needs to be a Text, we should use exactly the same code as ResourceInfoPage#createBasicInfoGroup(..).
(In reply to comment #18) > Created attachment 194721 [details] > Screenshot on GTK > > I had a bad gut feeling about the change from Label to Text in the > EditorSelectionDialog. When I tried it on GTK, I saw that the widget background > is now white on that platform. > > What was the reason for not using "new Label(contents, SWT.WRAP)"? That looked > good for me on Windows and GTK. If it needs to be a Text, we should use exactly > the same code as ResourceInfoPage#createBasicInfoGroup(..). Even in Mac the effect is similar. Without that change, the label doesn't wrap and the dialog is too wide, which is more uglier.
Created attachment 194749 [details] updated patch++3 (In reply to comment #19) > Even in Mac the effect is similar. Without that change, the label doesn't > wrap and the dialog is too wide, which is more uglier. I can't reproduce that. I tried it on the Mac HEAD/Cocoa and 3.4.2/Carbon, and they both look good with Label instead of Text (of course, we still need the layout changes). We should also include this fix. I've tested it on R3_4_maintenance on Windows 7, GTK, and Carbon.
The "updated patch++3" looks good. Released to R3_4_maintenance and the map file is updated.
