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 194411 - [EL] Expression language parsing extremely slow
Summary: [EL] Expression language parsing extremely slow
Status: RESOLVED FIXED
Alias: None
Product: javaee
Classification: Unclassified
Component: JSF Editor (show other bugs)
Version: 7.0
Hardware: PC Mac OS X
: P2 normal (vote)
Assignee: Marek Fukala
URL:
Keywords:
Depends on: 194534
Blocks:
  Show dependency tree
 
Reported: 2011-01-17 11:33 UTC by Marek Fukala
Modified: 2011-01-21 16:39 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
The snapshot (29.51 KB, application/octet-stream)
2011-01-17 11:34 UTC, Marek Fukala
Details
st (5.56 KB, text/plain)
2011-01-21 15:42 UTC, Denis Anisimov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Marek Fukala 2011-01-17 11:33:42 UTC
1) put the attachement http://netbeans.org/bugzilla/attachment.cgi?id=104990 into a web project
2) open it and do a keystroke
=> CPU load increases significantly for about 3-5 seconds.

This happens after each reparse. Caused by the ELHintsProvider. For more info see the attached snapshot.
Comment 1 Marek Fukala 2011-01-17 11:34:28 UTC
Created attachment 105062 [details]
The snapshot
Comment 2 Marek Fukala 2011-01-19 13:22:46 UTC
Most of the time is spent in WebBeanModel looking for the annotated classes. These leads to disk IO by Lucene which is quite slow. That wouldn't be bad if we do not run the 

WebBeanModel.runReadAction(new MetadataModelAction<WebBeansModel, List<WebBean>>()) many times in the row.

My question for Denis is: Is the WebBeansModel supposed to cache its content somehow so it does not always do the index queries? I think java indexing allows to listen on the source changes so I believe it is doable. 

If the caching is not done and cannot be implemented easily what I'll do is to compute and store the WebBeans in some reasonable context and the pass it where is was computed before.
Comment 3 Denis Anisimov 2011-01-19 13:37:11 UTC
WebBeanModel has the same structure as any other J2EE annotation based model.

It really do a cache for *type* elements ( java TypeElement ). Only first time
( when model is initialized ) the scanning is performed via Lucene indexing
and all consequent updates are propagated into the model via indexer listening.
This is true for each instance of model . Just make sure you use the same 
model ( It is possible to create *new* model on each request. The consequence
of such approach will be initializing of model each time when it is created . It 
leads to indexing ).

There is one thing which distinguish WebBeanModel from other J2EE models :
it also performs search for class's *methods* and *attributes* ( 
production methods and fields ). This Java elements cannot be cached because
there is no possibility to listen events from indexer about changes for 
such elements. The search of such elements are done each time on model request.
I believe it involve indexing .
Comment 4 Marek Fukala 2011-01-19 14:17:51 UTC
Thanks for the information Denis.
Comment 5 Marek Fukala 2011-01-20 13:21:42 UTC
The problem with multiple index access seems to be caused by the search over fields as Denis pointed out.

FINE [org.netbeans.modules.j2ee.metadata.model.api.support.annotation.AnnotationScanner]: findAnnotations called with javax.enterprise.inject.Model for [FIELD, METHOD]
FINE [org.netbeans.modules.j2ee.metadata.model.api.support.annotation.AnnotationScanner]: findAnnotations called with javax.inject.Named for [FIELD, METHOD]
FINE [org.netbeans.modules.j2ee.metadata.model.api.support.annotation.AnnotationScanner]: found element org.jboss.weld.literal.NamedLiteral
FINE [org.netbeans.modules.j2ee.metadata.model.api.support.annotation.AnnotationScanner]: found element org.jboss.weld.conversation.ConversationImpl
FINE [org.netbeans.modules.j2ee.metadata.model.api.support.annotation.AnnotationScanner]: type name mismatch, ignoring type org.jboss.weld.conversation.ConversationImpl, element init, annotation @javax.inject.Inject

Is dumped many times during hints computation for one page. Each call to AnnotationScanner.findAnnotations(...) involves index query.
Comment 6 Marek Fukala 2011-01-20 13:29:26 UTC
Pardon my ignorance, I have no expertise on these fields, cannot we do a simple caching by listening on general class source changes? So if any class hasn't changed since we last time searched over annotated fields and methods cannot we just return a cached value? Once there is *any* change fired by the indexer just throw it out and compute again. Even this involves many superfluous cache flushes it would still be far more better than computing it each time. Just an idea...
Comment 7 Denis Anisimov 2011-01-20 13:55:18 UTC
Is is possible but I don't think this is a good idea.
First of all : one need to add listener to ALL source files .
Because production fields or production methods could appear not just changed/removed.
There is no possibility to listen Java based changes. So FileObject listener
should be added to any java file.
Too  many listeners with need  to care about their clearance when file object 
becomes orphaned. 

So it's not good.

I would suggest to cache results on the client side based on atomic context.
Just as suggestion :
- add listener to the project folder ( I'm not sure if it observes events about
changes in the subfolder ) .
- Recall model method on each file object change event.

Model is not based on some special folder so it can't do the same 
internally .
Comment 8 Denis Anisimov 2011-01-20 14:16:07 UTC
This algorithm it seems should work good enough :
1) DataObject.getRegistry() -> DataObject.Registry 
2) DataObject.Registry.addChangeListener() add just one change listener to 
the data object registry .
3) On each event from listener call DataObject.Registry.getModified() 
4) For each modified data object check if they are java files and located 
in the required project .
5) Refresh cached value.
Comment 9 Marek Fukala 2011-01-20 15:00:42 UTC
I'm not sure whether the FS listening is a good idea since you will very likely got outdated data from the index in the moment of the FS change. I believe since the model is fully index based, you should listen  on the index, not on the underlying layers. 

It is not clear to me how come it is not possible to implement the caching only by the current events from the index. The index's context is a classpath, so if you change the classpath, you need to drop the cache. If *any* of the indexed java sources on the classpath changes the idexer will fire a change event. In such case you will drop the cache. What other kind of change could possibly make the cached data invalid????

I personally do not like the idea of FS listening at all so I'll likely do what I expected at the beginning I have to do and that is to store the computed values in some context at the client side.
Comment 10 Denis Anisimov 2011-01-20 15:34:13 UTC
>If *any* of the indexed
>java sources on the classpath changes the idexer will fire a change event. 
This is what I'm not sure.

First of all model is based on j2ee.metadata.model.support which gives the
framework which hides from model implementation index listening.
The latter framework care only about type elements ( the changes of Java Type 
elements itself not their inside elements such as fields or methods ).

So it is not possible to know when CONTENT of Java type element is changed.
But only when type element itself removed/added/ its hierarchy changed.

So I'm not sure that indexer allows to observes  changes of nested elements 
of type.

I will investigate this right know.
If it notifies about such changes then I really can implement the listener 
on my side without mentioned framework and cache returned value.
Comment 11 Denis Anisimov 2011-01-20 16:05:18 UTC
OK , I have checked the indexer events.
Indexer notifies its listeners about changes in the class content.

I will file a different bug for model enhancement because there could be
other client usage issues which lead to the behavior of the current issue.
Comment 12 Denis Anisimov 2011-01-20 16:10:50 UTC
See issue #194534 .
Comment 13 Denis Anisimov 2011-01-21 09:54:14 UTC
I have fixed the issue #194534 .

But I have noticed that Web Beans model is accessed each time on keystroke
in xhtml file. I don't know the details but this is not good.
As I understand you need only named elements from the Web Beans model and they
could appear only in specific context of xhtml ( f.e. as attribute value ).
But as I understand there is no check for context of keystroke. 
I've entered the space between elements attributes and  Web Beans model has been 
accessed.
Please note that each access to WB model is done inside Java source context lock 
( which lock a parsing access ). It could be a problem because there are many 
places which also uses the Java source context lock. Such lock will be blocked
until other thread ends its work. There are many performance issues respectively 
exactly Java source context lock.
So I would recommend to access to Web Beans model only when context is appropriate.
Comment 14 Marek Fukala 2011-01-21 10:48:21 UTC
Thanks Denis for the quick fix.

As for the model access upon each keystroke, I'm going to look at it now. This is definitively wrong, I thought the model is accessed only from the parsing.api tasks. I have to look at the code since Erno reimplemented it completely...
Comment 15 Marek Fukala 2011-01-21 11:02:52 UTC
Denis, please give such stacktrace where the model is access by each keystroke. 

From what I see the model is accessed only from the parsing thread upon a parser event. The EL error checking is context free so all the ELs from the whole file are evaluated even if you put a whitespace somewhere in the html text.
Comment 16 Marek Fukala 2011-01-21 13:16:36 UTC
OK. Lets say the original performance issue has been fixed by Denis'  fix of issue #194534 .
Comment 17 Denis Anisimov 2011-01-21 15:42:50 UTC
Created attachment 105244 [details]
st
Comment 18 Denis Anisimov 2011-01-21 15:44:06 UTC
See stacktrace in the attachment.
Actually it is reaction on placing mouse cursor before attribute inside element
( I've not even typed any character ).
Comment 19 Marek Fukala 2011-01-21 16:13:43 UTC
Since the GsfHintsProvider is a Scheduler.EDITOR_SENSITIVE_TASK_SCHEDULER task it should not be run just on the caret moved event. I reckon the parse happens only if you place the caret in the editor along with the editor gaining a focus, isn't it? In such case the parsing.api reruns all registered tasks.
Comment 20 Denis Anisimov 2011-01-21 16:39:08 UTC
Yes, probably it is on focus event handle.