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.

Bug 22422

Summary: Need a way to declaratively register property editors
Product: platform Reporter: Jaroslav Tulach <jtulach>
Component: Property EditorsAssignee: Jiri Rechtacek <jrechtacek>
Status: RESOLVED WONTFIX    
Severity: blocker CC: dkonecny, dstrupl, jglick, phrebejk, rkubacki, sdedic, tor, tpavek
Priority: P2 Keywords: PERFORMANCE
Version: 3.x   
Hardware: PC   
OS: Linux   
Issue Type: ENHANCEMENT Exception Reporter:
Bug Depends on: 29447    
Bug Blocks: 30660, 18273, 21676, 21979    
Attachments: Diff with changes in Utilities and with tests to check if it really works - needs JNDI in order to be useful
Implementation of XML registration that does not depend on JNDI

Description Jaroslav Tulach 2002-04-14 15:42:49 UTC
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.
Comment 1 Jaroslav Tulach 2002-04-14 15:44:35 UTC
This issue blocks part (3) of issue 21979
Comment 2 Jaroslav Tulach 2002-04-18 15:04:46 UTC
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
Comment 3 Jaroslav Tulach 2002-06-13 04:54:27 UTC
*** Issue 19899 has been marked as a duplicate of this issue. ***
Comment 4 Jaroslav Tulach 2002-06-13 04:56:40 UTC
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.
Comment 5 Jaroslav Tulach 2002-08-05 08:54:03 UTC
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.
Comment 6 Marian Mirilovic 2002-12-06 17:08:58 UTC
reassigne to Tim, new owner of property editors.
Comment 7 Jaroslav Tulach 2002-12-10 11:51:28 UTC
This would allow modules not to execute registration of editors on
startup. Adding perf keyword.
Comment 8 David Simonek 2003-01-02 16:22:21 UTC
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.
Comment 9 Jaroslav Tulach 2003-01-06 07:57:21 UTC
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.


Comment 10 _ tboudreau 2003-01-06 09:55:46 UTC
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?
Comment 11 Jaroslav Tulach 2003-01-06 12:33:42 UTC
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.
Comment 12 _ tboudreau 2003-01-09 12:43:00 UTC
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.
Comment 13 Tomas Pavek 2003-01-09 15:29:17 UTC
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...
Comment 14 _ tboudreau 2003-01-09 16:30:11 UTC
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).
Comment 15 Tomas Pavek 2003-01-09 16:50:17 UTC
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)?
Comment 16 _ tboudreau 2003-01-09 17:43:25 UTC
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?  
Comment 17 Tomas Pavek 2003-01-09 18:00:11 UTC
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...
Comment 18 Tomas Pavek 2003-01-09 18:03:22 UTC
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.
Comment 19 Jaroslav Tulach 2003-01-10 11:55:07 UTC
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.
Comment 20 _ tboudreau 2003-01-11 16:07:56 UTC
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?

Comment 21 Jaroslav Tulach 2003-01-12 12:42:14 UTC
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.
Comment 22 _ tboudreau 2003-01-16 18:12:49 UTC
Created attachment 8599 [details]
Implementation of XML registration that does not depend on JNDI
Comment 23 _ tboudreau 2003-01-16 18:19:33 UTC
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.
Comment 24 Jaroslav Tulach 2003-01-20 09:31:47 UTC
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).
Comment 25 _ tboudreau 2003-01-20 11:35:48 UTC
>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.


Comment 26 Jaroslav Tulach 2003-01-20 12:39:43 UTC
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.
Comment 27 _ tboudreau 2003-01-20 12:57:17 UTC
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.
Comment 28 _ tboudreau 2003-01-20 18:11:10 UTC
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?
Comment 29 David Konecny 2003-01-21 09:05:27 UTC
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.
Comment 30 Jaroslav Tulach 2003-01-21 15:37:51 UTC
"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.


Comment 31 _ tboudreau 2003-03-03 12:57:50 UTC
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?
Comment 32 Svata Dedic 2003-03-03 13:08:10 UTC
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.
Comment 33 _ tboudreau 2003-04-17 14:13:06 UTC
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?
Comment 34 David Konecny 2003-04-17 14:29:28 UTC
What you heard that made you think that registry impl might be less
suitable? :-)
Comment 35 _ tboudreau 2003-04-17 23:27:36 UTC
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.
Comment 36 _ tboudreau 2003-11-13 12:17:27 UTC
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. 
Comment 37 David Strupl 2003-11-13 14:13:47 UTC
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.
Comment 38 Tomas Pavek 2003-11-13 15:29:55 UTC
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.
Comment 39 _ tboudreau 2004-04-19 14:52:52 UTC
Passing property editor issues to new owner, Jirka
Comment 40 _ rkubacki 2005-09-16 13:36:40 UTC
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

Comment 41 Jesse Glick 2005-09-16 17:43:04 UTC
I think this might be WONTFIX - once we move away from using JavaBeans for
options display, this will become less and less important.
Comment 42 _ rkubacki 2005-09-16 18:04:04 UTC
There are still many SystemOption classes that use introspector to find set of
properties.
Comment 43 Jiri Rechtacek 2008-10-16 15:40:52 UTC
It's not planned for long time. It's fair to close as WONTFIX