This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Summary: | Suggest in layers: <folder name="..." ordered="true"> | ||
---|---|---|---|
Product: | platform | Reporter: | Jesse Glick <jglick> |
Component: | Filesystems | Assignee: | rmatous <rmatous> |
Status: | RESOLVED WONTFIX | ||
Severity: | blocker | CC: | jtulach, pnejedly, ttran, tzezula |
Priority: | P3 | Keywords: | API, PERFORMANCE |
Version: | 3.x | ||
Hardware: | PC | ||
OS: | Linux | ||
Issue Type: | ENHANCEMENT | Exception Reporter: | |
Bug Depends on: | 103187 | ||
Bug Blocks: | |||
Attachments: | A patch that modifies the DataFolders on SystemFileSystem to use original sorting provided by XMLFS |
Description
Jesse Glick
2002-02-08 13:24:35 UTC
Relieving XMLFS from parsing ordering attributes can speed it up by 100ms for the current set of modules which could mean by as much as 15-25% of the parsing time in the ideal case, see my todays' comments by #20168 We'll also need to change the XMLFS merging code to not sort the items by name and to keep the declared order during parsing as well. I've placed the sorting there only to help P.Hrebejk verify his platform related core layer split, it is itself nod needed. See issue 20168 It gets sticky to combine with XMLFS merging + caching, I guess. One merged folder might have ordered=true and the other not. Perhaps: 1. At *parse* time, <folder ordered="true"> generates just a flag in the folder ref object, and asking for file attributes returns them on demand. 2. At merge time, these flags are translated to real attrs, and if a folder with n files has n-1 ordering attrs, where they can be placed in a line, the folder is written out with ordered="true" (otherwise all attrs written out). 3. Folder ordering code optimistically looks first for a folder with n-1 ordering attrs that can be put in a line, and only if this is not true does it sort the hard way. It has been already realized that putting this datasystem stuff into filesystem layer is not good idea. Could we make the ordering functionality pluggable and implement it just in core.SystemFileSystem? Probably all we need to do is to give the creator of XMLFS ability to generate artificial attributes... Then issue #18829 could be generalized to making ordering pluggable (DataSystems API enhancement), where core supplies the impl with relative ordering constraints. I'm not sure how easy it would be to modify XMLFileSystem here; perhaps to keep the DTD clean, use a pseudo file attribute rather than the originally suggested attr on <folder>: <folder name="foo"> <attr name="ordered" boolvalue="true"/> <file.../> <file.../> </folder> then the core clients of XMLFileSystem would supply a "file convertor" permitting the 'ordered' attr to be converted to relative attrs (or some privately-named attr giving a list of files in the folder) at parse time, rather than merge time. Don't know how to handle cache merging this way however. OK, let's divide the problems into more subproblems. I have my idea so it is more targetted at it: At first, we need the XMLFS to keep the declared order during the parsing. Everything says and falls with this. We also need to keep the order during MFS propagation (the same file overriden on LocalFS have to be at the same position, but this may be no-issue because of OpenIDE-Folder-Order attribute). We don't need an attribute to say that the folder is sorted, do we? At second, we need to keep the order in the merged layer for the same reason, only one start later. Now we're done with not merged folders like component palette. (DF will just keep the ordering) At second and half, we have to add the layer information to the merged XMLFS. The layer information is lost during merging but will be needed for XMLFS autolocalization for looking up the right bundle (see issue 18310) The layer information can be stored as an attribute stating where the layer have changed, example: first.xml: <folder name="merged"> <file name="A"/> <file name="B"/> </folder> second.xml: <folder name="merged"> <file name="C"/> <file name="D"/> </folder> cache.xml: <folder name="merged"> <file name="A"/> <file name="B"/> <file name="C"/> <file name="D"/> <attr name="layerfrom:A" stringvalue="first.xml"/> <attr name="layerfrom:C" stringvalue="second.xml"/> </folder> <attr name="layerfrom:merged" stringvalue="first.xml,second.xml"/> The nice thing about this is that you have blocks that are presorted (there are implicit attrs A/B=true and C/D=true) and you can still change the position by ordering attrs (e.g. A/C=true, D/B=true will give you A,C,D,B) The bad thing about this it that you would have to cancel nonexisting attributes if you'd like to switch A with B from other module (branding), but still doable. The other bad thing (not connected with ordering) is that the XMLFS would have to understand the layerfrom: attributes to be able to properly return "layers" virtual attribute as it is returning now. Hmm, that are my ideas, the bottom line is to not trash the order of files to begin with. My observation1: Since Jesse implemented cache, we do not need any hooks into XMLFS we can do most of the generation stuff during generating the cache. My observation2: Of course XMLFS has to keep the order of children as they were read form .xml file My observation3: When generating the cache we may not need to use XMLFS.setURLs (...) but multiple XMLFS with just one URL. This will help to identify layer the FileObject is read from My observation4: If we continue to use setURLs (...) we can still identify the layer by calling FileObject.getAttribute ("layers"); My observation5: In order to satisfy issue 18829 the cache could add attributes according to the displaynames of the fileobjects... My observation #0: We will either need to store layers attribute for each file in the cache or have a way how to derive it from other informations. And placing the physical attributes in the cache is equivalent of leaving SFS.* attributes there. So the problem is not with merging/cache creation but rather with a later cached run. Oh, I think I see where you're heading to: The cache will not be a single merged XML layer but rather a storage of all the information from the XMLFS so it would contain, say, separate preparsed layers in a single file. ad 5) Do you think the cache impl would be able to reach the display names during its generation? And what about starting the IDE in different locale? (i.e. will the locale info be part of the cache hash?) A couple of notes: 1. If we implement the cache using a random-access binary DB file, we need not worry how much extra info we store per file. So we can premerge layers and still keep info about layer of origin without penalty; or we could leave semantics of SystemFileSystem.localizingBundle as it is without performance penalty. 2. Current cache design means that the hash is computed from actual URLs, so switching locale/branding will automatically recreate the cache. I don't understand purpose of Yarda's #2 BTW. If you have ordered="true" we need to keep info about that ordering, if not we don't; but even when we do keep info about the ordering, you need to consider merged ordering constraints from several layers anyway. Also if we implement a binary layer cache, XMLFileSystem would not actually be used at runtime at all, unless some module used it for some random purpose, or unless caching is off on some layer (e.g. userdir modules which currently get no layer cache, since they are in a different FS, for reasons still a mystery to me). I suggest to postpone Milestone to 4.0 because: 1/ original idea evolved and was fundamentally changed and not finalized yet Originaly was planned to add one attribute and keep ordering in XMLFileSystem as is (already implemented). Jesse, ad 2)
>2. Current cache design means that the hash is computed from actual
> URLs, so switching locale/branding will automatically recreate the
> cache.
Not true. If I don't have locale variants of layers (which is
true for all modules I know about), the the URLs will be the same
under different locales. Only SFS should use different bundle
to compute display names over the same content of XFS.
This is why you can't "add attributes according to the displaynames
of the fileobjects". Or did I misunderstood Yarda's intentions?
True, if you do not have a locale variant of the layer, the cache will not be changed according to locale. So any caching of display names has to use a different mechanism, I suppose. Is it really that important to cache display names though? Assuming we can make storage of the SystemFileSystem.localizingBundle attribute unnecessary or optimized for speed/memory, is there much advantage in caching the name? Only a handful of files will really be asked for their display names normally, so few bundles need be loaded for them; and I would only expect alphabetical folder ordering to be used for a few folders, and not generally during startup at all. Created attachment 5285 [details]
A patch that modifies the DataFolders on SystemFileSystem to use original sorting provided by XMLFS
So what, do you think that I should apply my patch? It guarantees that all files provided by one module are in correct order and thus I believe it is better than the current state. Let me know today if you do not believe I should integrate it. Go ahead. I think it is better than nothing. Till now, no particular order for layer files was specified if there were no ordering attributes... I commited Jarda`s patch with tiny change: name replaced with attrName. if ("OpenIDE-Folder-SortMode".equals (attrName) ... Checking in FixedFileSystem.java; /cvs/core/src/org/netbeans/core/projects/FixedFileSystem.java,v <-- FixedFileSystem.java new revision: 1.7; previous revision: 1.6 So, now I consider this bug as fixed. verified I guess I wasn't paying too much attention to this bug, but it is listed as fixed and even appears in the new features list, and I don't know what the fix actually was! Please explain: 1. What precisely is the new behavior, from the perspective of a module developer? [My guess: no clearly describable behavior.] 2. Is this documented anywhere? Listed in apichanges.xml? [My guess: no.] 3. Are there unit tests written to ensure it continues to work? [My guess: no.] 4. Does it work even with layer caching (which is on by default)? [My guess: probably not.] 5. Has it been tested heavily to ensure that it works correctly when more than one module adds to a given folder? I.e. if M1 and M2 both add files to folder F, M1 adding F1, F2, and F3, M2 adding F4 and F5, and M2 specifies F5/F1=true, then the system must ensure the order F4-F5-F1-F2-F3. Does it? Etc. [My guess: never been tried, doesn't work.] 6. Are there any actual modules converted to use this new feature that could be looked at for an example? [My guess: no.] usersguide would be an obvious test candidate since it has dozens of tips of the day which need strict ordering. Reopening until someone who knows more about this reads my previous set of comments and responds somehow... Jan Zajicek marked this VERIFIED but I see no further information about *how* it was verified, other than that a CVS commit was made. Also why does it matter to add this file attr to FixedFileSystem? Only a handful of folders in the SFS come from FFS. For example, Services/Hidden/, UI/Runtime/. If you have some random folder, e.g. <filesystem> <folder name="Foo"> <file name="x"/> <file name="y"/> <file name="z"/> </folder> </filesystem> what effect would this have? I just tried in a dev build installing a module with this layer: <filesystem> <folder name="foo"> <file name="one"/> <file name="two"/> <file name="three"/> <file name="four"/> <file name="five"/> <file name="six"/> <file name="seven"/> <file name="eight"/> <file name="nine"/> <file name="ten"/> </folder> </filesystem> In the Explorer in the system file system under foo/ I see: eight five four nine one seven six ten three two I.e. random order. Seemed to work for me. Do not know what happened. Update: with the binary cache manager, storing information from each file such as its origin layer should no longer be significant overhead. (Only paid when you access the file, and not much then.) Can reevaluate this RFE, and also the one re. not requiring SFS.localizingBundle. It was unfeasible under the XML cache manager to store an "origin" attr for every layer file, since that would cause the cache size to balloon, increasing load time significantly, as well as standing heap consumption. Probably obsolete due to issue #103187. |