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 24060 - form editor's event generation system
Summary: form editor's event generation system
Status: RESOLVED FIXED
Alias: None
Product: guibuilder
Classification: Unclassified
Component: Code (show other bugs)
Version: 3.x
Hardware: All All
: P2 blocker with 1 vote (vote)
Assignee: issues@guibuilder
URL:
Keywords: PERFORMANCE
Depends on:
Blocks: 26581 27741 34218
  Show dependency tree
 
Reported: 2002-05-27 10:06 UTC by pepeio
Modified: 2003-08-13 14:43 UTC (History)
1 user (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
patch with the new event system (347.50 KB, application/octet-stream)
2002-12-11 19:01 UTC, Tomas Pavek
Details
the patch, place in .../modules/org-netbeans-modules-form dir (346.10 KB, application/octet-stream)
2003-01-03 16:04 UTC, Tomas Pavek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description pepeio 2002-05-27 10:06:42 UTC
I've been using netbeans for more than one year now, and 
love it, but something annoys me about the
swong form generator.
It is well done, efficient and all, but the part i would 
like to discuss is about the code generated.
When i add an EventListener to a component, it creates an 
anonymous class. This is what annoys me. while
it is certainly very useful, it creates lots of small new 
classes for interfaces, and when you have big ones,
you end up with hundreds of anonymous classes. They eat up 
memory (there is a fixed class allocation that i
was told being around 4Kb), slow down startup and creates 
noisy directories.
Wouldn't it be possible to generate classes in an other 
form, that would remove those problems? I've got
an idea that can, but leads to code philosophy discussions 
that might be hot. (if that can be said...)
Image ine i have a JFrame with 10 buttons. Actually if i 
want to intercept Actions on those buttons, NB will
create 10 anon classes. If the JFrame was set to extend 
java.awt.event.ActionListener, the form could
generate a single ActionPerformed method in that same 
class that would then dispatch the messages to the
final method.
The cost would be almost the same, but the whole projects 
would have gained in clarity, loading time would
be smaller, and memory consumption shold be drastically 
reduced when having big GUIs.
Comment 1 Tomas Pavek 2002-06-04 16:57:58 UTC
The reasons to avoid anonymous classes generation are 
reasonable. But I don't think it should be done the way 
you suggest - by implementing listener interfaces in the 
form class directly. There are at least two problems:
- it's sometimes more effective to extend an adapter class 
than to implement whole listener (where only one method is 
used),
- the form superclass may already implement some of the 
interfaces (as you can create forms from any JavaBean 
class) - then you would override its methods.
Comment 2 pepeio 2002-06-04 18:05:36 UTC
The first problem is not a problem. We can't make the 
class to extend something else than what it already 
extends if it does, and implementing is the only way to 
give it the possibility to catch itself the messages it 
needs. On an other hand, the form  editor could create 
a 'patternized' class that would handle all needed events, 
but that would not solve the problem of the amount of 
classes. While it would reduce the number of anonymous 
classes, we'll still get twice the number of classes 
created.

For the second problem, isn't it why the super() methods 
were done for?
Calling super() at first won't solve the overriding 
problem ? To me, it seems it will.

Of course, extending of all needed events and adding event 
handling inside the class is not a common thing for 
netbeans/forte users, but that's the only one i found that 
would solve all practical problems.
Comment 3 Marek Grummich 2002-07-22 08:13:02 UTC
Target milestone was changed from not determined to TBD
Comment 4 Tomas Pavek 2002-09-27 10:32:13 UTC
This could be addressed by performance improvement efforts in NB 4.0...
Comment 5 pepeio 2002-10-13 18:44:15 UTC
Sorry, i'm not sure i correctly understood what you said.
Does it mean that this problem would be solved by a 
performance increase in an other version?
Comment 6 Tomas Pavek 2002-10-15 12:27:03 UTC
I meant that we could really implement this for NB 4.0, as it is a
performance issues, which we are focused on.
Comment 7 pepeio 2002-10-25 14:42:01 UTC
Performance is only one, and the last point of the problems i listed
about the actual event generation system. 
to reformulate, the actual system:
- eat lots of memory
- adds lots of noise to applications by creating lots of anon classes.
Noise is added in sources and file system.
- class loading takes time. That time can be avoided.
- whatever the solution used for increasing speed of netbeans, it will
not solve the problems of generated applications.

When having big GUI, class loading is still a problem because of those
hundreds of classes. I don't care so much if Netbeans takes time to
start. i do it once a day, and having some more seconds of wait is not
big in the looong starting time. But, for generated applications, that
can be started many times a day, where user has low memory, low
processor, and slow hard drive that is a HUGE quality problem. I won't
tell my users to upgrade their machines because of a coding problem.
Oh, maybe i'll have to tell them to start the application through
netbeans?? :(

<----QUOTE------>
- it's sometimes more effective to extend an adapter class 
than to implement whole listener (where only one method is 
used),
<----QUOTE------>
effective in what? Would you dare to say that having those anon
classes take less time and memory than implementing listeners in the
GUI classes? The actual method is a hog in both domains. 

<----QUOTE------>
- the form superclass may already implement some of the 
interfaces (as you can create forms from any JavaBean 
class) - then you would override its methods.
<----QUOTE------>
Well, then on the exit of the super call, verify if the event is
consumed or not. How would that cause any problem?
Comment 8 Tomas Pavek 2002-10-25 15:04:11 UTC
Frederick, nobody is arguing with you. What I try to say 
is that this is exactly what we want to do - change the 
event generation code style the way you suggest (or 
similar). The performance aspect is in that we'll be able 
to regenerate all forms we use in NetBeans, so then our 
dialogs will be faster. And of course this will apply to 
any forms created in NetBeans... Okay?
Comment 9 pepeio 2002-10-25 16:34:03 UTC
Sorry if there is a problem in me understanding what you say (not a
native english talker), but the "as it is a
performance issues" you wrote tend to let think that you saw the post
as only a perf problem. I was just stating it was not. This is a very
sensitive problem to me and i really wish to see a proper solution.

So what solution do you have in mind?
What about the arguments i just exposed?
Comment 10 _ pkuzel 2002-10-25 21:16:45 UTC
For JDK 1.3 target platform proxy API can be used i.e. generate a
delegator.

It requires target platform knowledge that can be provided by NB 4.0
projects.
Comment 11 _ viendu 2002-10-29 15:55:15 UTC
How about using the following method:

Create an inner class just like now for all events.  So the difference
is that now there is 1 class for event, not many.  This class will be
shared by all events in the form.  This will solve the extra work
required by super(), and also reduce the number of files needed to
load when a form is instantiated (potential performance gain).  Yes,
current implementation give a theoretic faster response time to event,
but give a much slower instantiation and bigger memory foot print. 
However, for a practical view (versus theoretical), perceived response
time will not be improved at all to users (the if/else or switch()
will not slow down the event delivery) due to the fact that events are
not deliver madnessly, but only one or twice a second.  If there is
thousands of events in a form, there may be a little difference.  But
you can always generate code in a way that a binary search is used
instead.  I think using switch(), the compiler already generate a very
efficient code (hashing) for jumping, so there would not be much
theoretical differences.
Comment 12 pepeio 2002-10-30 08:12:03 UTC
> This will solve the extra work required by super()
In a jitted environment, there is no extra work if the method is
empty, as the call will be removed. (and reincluded if the super class
changes), so we don't have to worry there.
> (the if/else or switch() will not slow down the event delivery)
Should be true. jitted environments can optimise the switch() and
(some) if() by inlining the complete logic path, so there can
eventually be no cpu cost (only ram)

Anyway, those points are moot anyway, due to the few event frequency.
It could be way slower that we should not percieve the difference anyway.

I still stick to the automatic implementation of the needed event
listeners method. Can someone give me GOOD arguments that would make
it a bad solution?
Comment 13 Tomas Pavek 2002-11-04 09:31:01 UTC
I think that generating just one inner class handling all 
events (implementing all listeners used in the form) is 
the way to go. It will eliminate all the anonymous 
classes, and the performance cost of additional event 
dispatching will be quite minor for typical UI events. We 
may have the code generation style configurable, so the 
user could choose also the "old" anonymous innerclasses.

I prefer to have a special inner class, rather than 
handling the events in the form main class, because it is 
IMO cleaner and better manageable solution - not 
interacting with user code (not-guarded code), wich would 
bring potential consistency problems (and would be also 
much harder to implement).

As for using the Proxy, I think it does not bring any 
benefit over the one inner class listener, just creates 
lots of rather complicated code...

Comment 14 Torbjorn Norbye 2002-11-05 17:14:17 UTC
I agree with not using proxy; I think Yarda said on nbdev that it was
slow, but more importantly, the code generated by the form module
should not be "magical", it should be code most Java developers would
understand, AND, something they might have written themselves. I don't
think anybody in their right mind would write proxy code to handle
events. I think most developers will implement the listener interface
directly in their class, e.g. implements ActionListener, and then have
an actionPerformed() method which if/else's on the event source:
  if (evt.getSource() == okButton) { // handle OK
  } else if (evt.getSource() == cancelButton) {
     ...
  }

There are two problems with using this in the form builder:
(1) Unlike what somebody said, calling super.actionPerformed()
    if the parent class implements ActionListener is NOT 
    safe.  The parent code may not have realized it would be
    subclassed, so it might have done this:
      dismissButton.addActionListener(this);
      ...
      public void actionPerformed(ActionEvent evt) {
          window.hide(); // dismiss button was pressed
      }
    In other words, it's not checking the source of the event since it
thinks there is only one possible reason actionPerformed could be called.

(2) If there are a LOT of events, an if/then/else approach is not what
a developer would use (especially if it's frequently called code, not
say menu button invocation code), so the form builder would have to do
something different in that case in order to generate
"professional-looking" code.

I think the inner class approach is the best tradeoff. If you're going
to make it optional (E.g. the developer can choose whether to use the
old anonymous inner class approach or the new inner class) if it's not
too hard, having a third option where I can directly implement the
interfaces would be handy; this is the approach I would choose in most
cases (because it's the leanest code). Having a dialog which warns
about the possibility that you may be unintentionally overriding the
interface methods and to stick with the default if you're not sure
about what you're doing, might be a good idea.
Comment 15 Tomas Pavek 2002-11-05 17:41:29 UTC
I agree this would be nice. But the third option would be rather
complicated to implement - as the code (at least the "implements"
section of the main class) would have to be generated to user code,
not to guarded sections, which would bring up potential consistency
problems and conflicts... Well, probably not unsolvable, but
definitely more work.
Comment 16 Torbjorn Norbye 2002-11-05 18:23:27 UTC
I think it would be acceptable (for users who chose the third option)
to make the actionPerformed (& friends) declarations guarded sections.
Or are you referring to the "implements ActionListener" part in the
class declaration line? Since this is an expert option I don't think
this has to work very smoothly. E.g. this is for experts who want
their code lean and they're willing to add "implements ActionListener"
to the declaration line themselves. 

In other words, this is intended for people who care a lot about the
generated code quality and performance and are willing to go the extra
mile. Goes with the "common is easy, advanced is possible" guideline.
Comment 17 pepeio 2002-11-18 13:34:48 UTC
I think we've been to a point where the needs and possibilities of
solutions are explained. Cool. I'm happy.
Nevertheless, i would like to point something on sir Tor Norbye's
comment. The first example you gave, that shows an unsafe call of
super is not on the right target.
Here, what is unsafe is the original actionPerformed method. Anyone
extending the object and overriding actionPerformed will encounter the
problem, thus, it's the method that is unsafe and not the super call.
If the method were to correctly look who's the event for, it would
behave correctly.
So, i can say that using the super calls would reveal bad code, and
problems would only be the fault of the programmer of the super class.
One more good point for extending listeners in the main class. :)

About the second point, the JIT can optimise really well a suite of
if/else, so there is nothing to worry about them (except the visual
aspect of the code).
Comment 18 Tomas Pavek 2002-11-18 14:01:27 UTC
You wouldn't say this if you sow so many mistakes in 
users' beans and had to answer all those questions I 
did... It does not matter where the bug is, people will 
always blaim the form editor: "It worked with the last 
version and now it does not!"
You may look e.g. at issues 27185 and 27186 - we are 
forced to go back with workarounds for bad beans/forms 
that we stopped permitting... So I really want to be very 
careful before introducing any functionality that could 
reveal some user errors in user code.
Comment 19 Tomas Pavek 2002-11-18 14:05:39 UTC
> ... if you sow so many mistakes in ...
Nice typo ;) Should be "saw so many mistakes" of course.
Comment 20 pepeio 2002-11-18 17:09:42 UTC
I'm sorry that people are so inconcious ( and stupid? ) to blame
others on their faults, but imho 'you' (whole netbeans) should not let
the users dictate for ethically bad solutions. For the quality of the
whole java world, this should not be permitted. (and that includes
myself, if the solutions i propose are proven to be bad.)
It's the benefit of everyone to forbid bad or dangerous code.
Until one doesn't break code, one should not fear remarks. Or maybe
the faulty people feels ashamed and don't want to show it.

I understand your pain, but i don't understand the reasons to accept
that. hmmm.. getting OT... 
Comment 21 pepeio 2002-11-18 17:23:34 UTC
mhh. as i read from the two reports you submited to me, these bugs are
somewhat different. Here, the comportment of netbeans changed, because
something related to your architecture and system changed. Even if it
is a bug correction, they should not be able to create bad beans. Your
suggestions are full of good sense.
In my post i was talking about a problem that is not related to
netbeans, but is a programming error in general.
You should not care about users reporting a bug because of the
overridden method problem. It's their fault, not yours, and they
should thank you because you help them maling better code. (okay, not
so, but i think you'll get the idea..)
Comment 22 Tomas Pavek 2002-12-11 18:59:58 UTC
I've created form_events_40 branch for reworking the event management
and code generation. A working prototype is already done, I'm
attaching a patch. You can try it - just create a directory named
org-netbeans-modules-form in modules/patches and place the
form_events_patch.jar in it. The generation style of listeners can be
set in form editor's Options (Listener Generation Style property).
Note the option "Main Class" is not fully implemented yet (does not
generate calls to super and does not modify the form class declaration
to implement required listeners).
Comment 23 Tomas Pavek 2002-12-11 19:01:39 UTC
Created attachment 8271 [details]
patch with the new event system
Comment 24 Tomas Pavek 2002-12-12 14:28:50 UTC
Before merging this to trunk, we should do complete tests of the event
system (attaching events, removing, renaming, undo/redoing,
saving/loading).

Would be nice to test also the "pathological cases" like:
- event listener is not interface, but normal class,
- event method has not EventObject parameter or has more than one
paramater,
- event method returns something,
- event meyhod throws exceptions,
- add listener method throws exceptions,
- there are two listeners having a method with the same signature.

These cases are not normally seen when working with decent beans, but
should be taken into account, especially when generating one
innerclass listener.
Comment 25 pepeio 2002-12-13 07:15:03 UTC
let me guess.... does not work under releases other than 3.4? (dumb
question of someone using sunOne... :)
If not, no such setting there.
Comment 26 Tomas Pavek 2002-12-13 10:50:41 UTC
Well, this is supposed to work/be tested on current trunk development
builds...
Comment 27 Tomas Pavek 2003-01-03 16:02:40 UTC
I've done some more work in form_events_40 branch, I think it's
completed now. I'm attaching new patch, best trying with 20030106 (or
newer) dev trunk build.
Comment 28 Tomas Pavek 2003-01-03 16:04:51 UTC
Created attachment 8430 [details]
the patch, place in .../modules/org-netbeans-modules-form dir
Comment 29 Tomas Pavek 2003-01-23 17:15:42 UTC
Integrated into trunk, available in tomorrow dev build.
Comment 30 dgriff 2003-08-13 14:43:38 UTC
This issue, to me, suggests a return to JDK 1.1 style event 
handling where one method handled the events and dispatched 
an individual event to the correct code to handle it. The 
dispatcher was an if else chain or switch like structure. 
In JDK 1.2 we where provided with inner classes to get rid 
of the dispatcher code and enable a cleaner event model. ( 
This history is from my memory so some of the details maybe 
wrong. ) The Sun engineers recommend using inner classes 
over the dispatcher that I think is being suggested here. 
The JDK 1.4 has a java.beans.EventHandler. This class is 
intended to replace most common uses of inner classes in 
event code. This class provides most of the things Tomas 
Pavek wants. Significantly 
less classes loaded and clean directories. Because the 
class is used where an inner class was used the memory 
footprint for a running application will probably be about 
the same. The startup time will be less if the class 
loading time is greater than the EventHandler constructor 
time (may need testing). 
I think Netbeans needs all three styles of event 
dispatching to handle Projects, JDK 1.1, JDK 1.2 and JDK 
1.4 .