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.
As part of my work on issue 21676 I have found out that a lot of modules registers their property editors in ModuleInstall.restore like: PropertyEditorManager.registerEditor (String.class, myEditor.class); there seems to be no other way to do it and this forms a serious performence bottleneck because it uselessly unitializes module classes on startup. I suggest to design general way for registration of propertyeditors that could replace the current one, was declarative and also worked for formeditor (needs to get all registered editors for one class). In the same way we should probably solve the beaninfo problem too.
This issue blocks part (3) of issue 21979
Created attachment 5470 [details] Diff with changes in Utilities and with tests to check if it really works - needs JNDI in order to be useful
*** Issue 19899 has been marked as a duplicate of this issue. ***
This is my proposal for declarative property editor and beaninfo registration. Please note that this also allows future implementation of XMLBeanInfo/XMLPropertyEditor info, because it uses layer to instance converting capabilities of datasystem.
Ales, this issue affects the scalability in # of modules which is very important. Without this some modules must execute some code on startup which is bad. I believe P2 is more appropriate especially if the code is already written.
reassigne to Tim, new owner of property editors.
This would allow modules not to execute registration of editors on startup. Adding perf keyword.
Seems important to me (agree with Yarda) - adjusting prio to P2. Yardo, is it possible to measure maximum potential time save when modules in standard nb distribution will all use declarative technique? I'm not familiar with modules API and don't know where to put measuring points. Thx.
Maximum save <= time spend in ModuleInstall. Disabling ModuleInstalls is described in issue 21676. Please be aware that this issue is a necessary step to avoid use of ModuleInstalls, but implemented itself will not have such big impact, I am affraid.
FYI, the hooks for this are in place to do this in my property sheet rewrite, I just haven't done any coding on it yet. It shouldn't be hard - the main thing will be that we do not want the performance hit of going off and playing with XML every time a property editor is requested (since this is done inside a paint loop). Not hard to solve - populate the property editor set the first time any property editor is registered, and listen for changes on the directory they're registered in. This, however, *would* create a problem (or at least a zombie classloader warning) if a property editor is deregistered; the most efficient way to handle XML registered property editors would be to register the XML property editors with java.beans.PropertyEditorManager - that means addition will work, but deregistering won't. Big deal or not?
Effecient cache should be part of core/naming module. The necessary thing is then to find a way how to communicate with the cache that a result should not be GCed, but that is not as hard.
Question, from a conversation in the office yesterday: Is it a requirement that a call to java.beans.PropertyEditorManager.findEditor(SomeClass.class) return an appropriate property editor (so the XML impl should register the editors into PropertyEditorManager). Note that with the property sheet redesign, calls to Property.getPropertyEditor will be much more frequent (in the old property sheet, a component is holding a reference - in the JTable implementation, the JTable renderer rubber-stamp approach is used). So either properties should cache this once created or any call to get a property editor should be extremely fast, because it will be done inside the paint loop.
Form editor would prefer "yes" answer to the question above. As for not holding property editors in property sheet - it could be serious performance problem - as common practice is just to create new instance in Property.getPropertyEditor(), I think nobody bothers with caching...
Okay, so the new implementation will keep a list of registered class names; when an editor is resolved that is registered by XML, the NbPropertyEditorManager will register it with PropertyEditorManager; subsequent requests for the same editor class will delegate to java.beans.PropertyEditorManager; NbPropertyEditorManager will listen to the folder of XML editors, and if a module is disabled, unregister the editor. Note: This *does* mean form editor code *must* be changed to use NbPropertyEditorManager, not java.beans.PropertyEditorManager - otherwise there is no chance for the XML declared editor to ever be registered. So maybe the delegation isn't a great idea in the first place - for everything to work with PropertyEditorManager, NbPropertyEditorManager will have to read its folder and register everything on startup - which means we get rid of the ModuleInstall bit but still do the work on startup. That might be acceptable *if* the work is done on the first request for a property editor from NbPropertyEditorManager; however, it is meaningless if anything calls it before startup is complete - this needs to be checked. So if there is a node selection during startup, it can get called. Re the performance issue, I profiled my propertysheet revisions in OptimizeIt and one of the hotspots was indeed PropertyEditorManager.findPropertyEditor(). However, it probably won't break anything to modify Node.Property.getPropertyEditor to softly cache the result the first time it's called (weak caching would just get collected immediately, when the renderer lets go of the property editor).
Well, if the NbPropertyEditorManager must be used explicitly anyway (or you had to read the editors on startup), then we probably should not register the editors in java.beans.PropertyEditorManager. I just thought I could stay with PropertyEditorManager, but it should not be a problem. BTW I'll need similar registration facility for form editor's private property editors (i.e. registered from outside, but used only in form editor, not in the whole IDE). Is there a chance to use these stuff somehow for this purpose too (so not implementing the same in form editor again)?
You could register them with PropertyEditorManager (just don't do it during startup) but if some other module registers a conflicting editor with NbPropertyEditorManager, then that's the one that will show up on a standard property sheet. If the form editor needs to register property editors which are only used by the form editor, couldn't you just return what you want from Property.getPropertyEditor() for your properties, and don't register them at all?
To the first: form editor just needs to obtain all possible editors for given property (which is more complex than property sheet does as it uses just one editor). Form editor collects all the editors and creates one wrapper property editor for them. So the only requirement on any property_editor_manager is to provide registered property editor in the IDE for given type. Now I see the right way is probably to do it via special NbPropertyEditorManager - as filling the java.beans.PropertyEditorManager "sometimes not during the startup" looks strange. If I get everything from NbPropertyEditorManager, you need not bother with moving that all to java.beans.PropertyEditorManager. There could be problem if someone else rely on that, but that's probably very rare case. To the second (form editor's private registry): Cannot return it from Property.getPropertyEditor - that does not work for general type (used in any property of given type) and for other modules. E.g. i18n module needs to register its special editor in form editor for String type...
So if I said "Form editor would prefer yes..." above, I've changed my mind - it would be nice I had not use new interface, but in fact it's no big problem.
The patch can easily obtain more than one property editor for a given Class. Such functionality just is not public. Also, if form editor needs different registry for editors, it is simple to do. Again it is just not public contract. The question seems to be whether try to come with public API or form editor just accesses the editors via JNDI itself.
Jarda, question: How attached are you to using JNDI for this? At this point I don't know JNDI well (but I feel I am about to learn). How difficult would it be to attach additional registration info and retrieve it? Some performance optimizations are possible if required registration includes the following information: - If the custom editor is a Window subclass - The preferred size of the custom editor (at standard fonts, 72dpi - diffs can be calculated easily to compensate for nonstandard font sizes) Note that all this fetching of property editors may be done multiple times in a paint loop - it needs to be very very fast. Also, there are a couple requirements here: - There are two use cases for property editors: 1.For rendering (nothing will attach a listener, the editor is created to get a value and possibly paint it) 2. For use to actually edit a property I would prefer to do something like: findEditor (Class class, boolean newInstance); to cover both use cases - for 1, return a cached instance. I can't cache instances in the property sheet because a Node.Property might override getPropertyEditor() to return something other than what my cache would contain for the value class - so this has to be in the provider of the property editor. Any objections to this?
I have problems imagining different impl that would not use JNDI and would be as flexible. By using JNDI you allow others to define declarative editors and beaninfos because the JNDI delegates to the full machinery of system file system. I suggest to stick with it.
Created attachment 8599 [details] Implementation of XML registration that does not depend on JNDI
From offline discussion: >> It is not complicated, but I agree that the impact is unclear. Moreover it >> depends on core/naming module. Which still is not in trunk builds (as far as I >> know). >if so then indeed we shouldn't put in in Nevada/3.5 I've attached an implementation that does not depend on JNDI. Since the use of JNDI is an implementation detail not exposed to users of the API, it can be converted later if there is a compelling reason. Please review. BTW, ignore the removed call to allItems() in Node.java - it is a fix for something else.
The implementation using NbPropertyEditorManager better separates the API from the impl. That is fine. Maybe rename NbPropertyEditorManager to ExPropertyEditorManager? Concerns still exists: 1. "JNDI is an implementation detail" is and is not. Your proposal changes the way how editors are registered and that is also API. So this has to be stabilized for future and has to be fast and capable. 2. Your way of registration is not as capable as the previous one. Moreover is non-standard, does not use common registration schemes on SFS. 3. Is your registration faster? Or what is reason for choosing this style? Moreover there is a small misunderstanding. What you have implemented is XML registration, but the description of the PropertyEditor itself cannot be described in XML. Which was goal of the original implementation (not to do it, but to allow it).
>The implementation using NbPropertyEditorManager better >separates the API from the impl. That is fine. Maybe >rename NbPropertyEditorManager to >ExPropertyEditorManager? Yes, some name that is not so long would be good. ExPropEdMgr? >Concerns still exists: >1. "JNDI is an implementation detail" is and is not. Your >proposal changes the way how editors are registered and >that is also API. So this has to be stabilized for future >and has to be fast and capable. Either proposal changes the way property editors are registered and is API. Agreed it needs to be fast and capable - I worry that the performance characteristics of JNDI are more likely to change over time than those of a HashMap. >2. Your way of registration is not as capable as the >previous one. How? Are both implementations going after the same goal? >Moreover is non-standard, does not use common registration >schemes on SFS. This is true - it is not using .instance files or other conventions we have - the file name is not used for anything. Probably the proposal could be simplified or improved in that regard. >3. Is your registration faster? Or what is reason for >choosing this style? Using JNDI to do it adds much complexity to the implementation without adding so much value (users of property editors will never know or care how it is implemented). If we want to make the goal XML registration of BeanInfo classes, then perhaps it makes sense. If so, create an issue for that and make this issue depend on it. If we are only trying to declaratively register property editors, it seems like killing a mosquito with a sledgehammer. >Moreover there is a small misunderstanding. What you have >implemented is XML registration, but the description of >the PropertyEditor itself cannot be described in XML. >Which was goal of the original implementation (not to do >it, but to allow it). I do not understand the above paragraph - please clarify.
I've made this issue block the issue 18273 that requests declarative beaninfos. I think that the main problem is "Using JNDI to do it adds much complexity to the implementation without adding so much value" I cannot agree with the second part and I think the first one is also not quite correct.
Well and good. So if we will be going with Jarda's original approach, using JNDI, what shall we do with this issue for now? It is a P2, I am supposed to do something about it, and the last conclusion that I heard was that the JNDI approach cannot be used until JNDI support is available in the builds. Petr Hrebejk mentioned that there *is* JNDI support available now. Is that enough? I would like to make progress on getting *something* integrated that resolves this issue.
Okay, Jarda: Can we agree on what the XML registration should look like in the SFS? Then, if we cannot use JNDI immediately, we can use a modified version of my implementation until JNDI is available. If we change the implementation to JNDI based later, it will be transparent to users. Or can we use JNDI immediately?
Sorry for jumping late. As owner of naming module I should probably follow this issue. I skimmed your discussion, read Yarda's propertyeditor.diff and have a question on Yarda: why do you think we have to use JNDI here? Should not the usage of systemFS be sufficient here? From the patch it seems to me that it should - it just opens folder ("/Providers/Editors/"+propertyClass.getName().replace('.','/')) on systemFS and from this folder it uses first found instance. Or am I missing something? Otherwise I tend to agree with Tim's last comment: once the JNDI is agreed to be used in NB we can easily change the implementation. Till that time we should not create dependencies on JNDI.
"once the JNDI is agreed to be used in NB we can easily change the implementation" - true, if you can keep the semantic and without JNDI that will be hard. Of course I am assuming that we want to do the DO conversion and that means to copy following class: http://www.netbeans.org/source/browse/core/naming/src/org/netbeans/core/naming/Utils.java?rev=1&content-type=text/x-cvsweb-markup and I really cannot understand why we have to copy this again and again and again instead of including the core/naming module in the build and just sharing the code.
FYI, as part of some other work, I patched PropertyEditorManager with some logging code. The following is the results for the default configuration. (Note IntEditor and BoolEditor don't exist in the trunk yet, so really it's 8, not 10 editors explicitly registered on startup). EXPLICIT REGISTRATIONS: REGISTERED BY CORE CODE (in NonGui.java): Registered editor for char with class org.netbeans.beaninfo.editors.CharEditor - 0 Registered editor for class [Ljava.lang.String; with class org.netbeans.beaninfo.editors.StringArrayEditor - 1 Registered editor for class [Lorg.openide.loaders.DataObject; with class org.netbeans.beaninfo.editors.DataObjectArrayEditor - 2 Registered editor for int with class org.netbeans.beaninfo.editors.IntEditor - 3 Registered editor for boolean with class org.netbeans.beaninfo.editors.BoolEditor - 4 REGISTERED BY MODULES: Registered editor for class org.netbeans.modules.autoupdate.AutoupdateType with class org.netbeans.beaninfo.editors.ServiceTypeEditor - 7 Registered editor for class java.rmi.activation.ActivationGroupDesc with class org.netbeans.modules.rmi.activation.ActivationGroupDescEditor - 8 Registered editor for class java.rmi.activation.ActivationDesc with class org.netbeans.modules.rmi.activation.ActivationDescEditor - 9 MODIFICATIONS TO THE EDITOR SEARCH PATHS DURING STARTUP: org.netbeans.core.NonGui.run(NonGui.java:354) org.netbeans.core.compiler.Install.restored(Install.java:52) org.openide.debugger.module.Install.restored(Install.java:41) org.netbeans.core.execution.Install.restored(Install.java:86) org.netbeans.modules.xml.tax.TAXModuleInstall.installBeans(TAXModuleInstall.java:63) So basically, it appears 5 things register property editors or editor paths on startup: autoupdate, core, debugger, core-compiler, core-execution, rmi and tax Question: This isn't really a lot of stuff. Is it worth it to go to great lengths to fix this?
I think the more important data is how many Node.Property have an explicit code for constructing a PropertyEditor instance. Many of them could use a query mechanism to locate a suitable PE for its data - if there was such mechanism, of course.
Okay, so now it looks like no JNDI in core. What now? Also, it sounds like registry impl might be less suitable for XML beaninfos. Yes? No?
What you heard that made you think that registry impl might be less suitable? :-)
Nothing, but it would be nice to be able to form a plan, and to assess where this sits in the priority scheme of things.
Clearing STARTED status on this issue - looks like the earlier solutions won't be the ones that work out. Question: Is there any reason modules can't simply include static { //register some property editors, directories, whatever } in some class that might need them? They'd still need a shutdown hook to unregister, but that's less of a problem.
Performance people please correct me but shouldn't a module that is never invoked by the user add anything to the startup time? Imagine 100 modules each loading one class doing something during the startup sequence. Something that is never seen/needed by the user. The classes should be loaded only if someone asks for them AFAIK. Also if you would reference the prop eds from your startup class they would be loaded as well ... My estimate is you could easily end up loading several hundred classes more than needed.
From another point of view: > Is there any reason modules can't simply include > static { > //register some property editors, directories, whatever > } > in some class that might need them? would not work in case a module (or say the user directly) needs the property editors provided by another module. The only solution is they are available always - either registered during startup using the standard jdk mechanism, or in layer using some other mechanism.
Passing property editor issues to new owner, Jirka
Yet another proposal: Use Provides: attribute of manifest fiel with a token like BeanInfo or BeanInfo<path> in module manifest to declare that a module provides bean infos and process these in a module system once during startup (before ModuleInstall after reading module list or setting classpath). Similarly for editors. Pros: eliminates parts of module installs so we save some loaded classes and we set these values only once Cons: it is a hack as it adds a feature to module system that was not originally planned here BTW: currently there are following beaninfo directory in our codebase - beans/src/org/netbeans/modules/beans/beaninfo collab/ui/src/org/netbeans/modules/collab/ui/beaninfo core/src/org/netbeans/beaninfo core/execution/src/org/netbeans/core/execution/beaninfo form/src/org/netbeans/modules/form/beaninfo java/srcmodel/src/org/openide/src/beaninfo xml/tax/src/org/netbeans/modules/xml/tax/beans/beaninfo
I think this might be WONTFIX - once we move away from using JavaBeans for options display, this will become less and less important.
There are still many SystemOption classes that use introspector to find set of properties.
It's not planned for long time. It's fair to close as WONTFIX