Bug 169991

Created attachment 90702 [details]
In progress diff

M csl.api/nbproject/project.xml
M csl.api/src/org/netbeans/modules/csl/spi/DefaultLanguageConfig.java
M html.editor/src/org/netbeans/modules/html/editor/gsf/HtmlLanguage.java
A csl.api/src/org/netbeans/modules/csl/impl/LanguageProcessor.java
A csl.api/src/org/netbeans/modules/csl/impl/NoDeclarationFinder.java
A csl.api/src/org/netbeans/modules/csl/spi/LanguageRegistration.java
A csl.api/test/unit/src/org/netbeans/modules/csl/spi/LanguageRegistrationTest.java

Once fixed, back out 270d0d7fca12.

Integrated into 'main-golden', will be available in build *200912020200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/270d0d7fca12
User: Pavel Flaska <pflaska@netbeans.org>
Log: #169991: Languages should not be loaded - temporarily allowed. Rollback once the bug is fixed.

Seems pflaska is working on this in

http://hg.netbeans.org/ergonomics/rev/annotation-based-csl-169991

?

BTW I see you have declarations like this:

public Class/*<? extends OccurrencesFinder>*/ occurrencesFinder() default Void.class;

For best type safety and clarity you may want to write it as:

public Class<? extends OccurrencesFinder> occurrencesFinder() default NoOccurrencesFinder.class;

where NoOccurrencesFinder (if possible in an impl package) is assignable to OccurrencesFinder but does nothing (all methods assert false), and the processor would check for this value and skip the registration.

The declaration is prepared to work in a way you suggested, unfortunately there is a javac bug in 1.6, the code is not compilable. (Once I find it I will attach the bug id.) It works in 1.7.

The bug Pavel talked about is:
http://bugs.sun.com/view_bug.do?bug_id=6512707
fixed in JDK7 and OpenJDK6. I realized it might be possible
to workaround the bug on NB side (the bug is caused by insufficient
cleanup, and I think it should be possible to create an annotation
processor that would perform the missing cleanup instead of
the compiler). But, if we could ensure that we always build 
with a compiler that does not contain this bug, I wouldn't need
to implement this workaround. Is there a chance that we would
always use our own copy of javac to build NB (like we currently
do on JDK5), or should I try to create the workaround?

(In reply to comment #7)
> Is there a chance that we would
> always use our own copy of javac to build NB (like we currently
> do on JDK5)

No. When Ant is run on JDK 6, we do not override javac. Maven users would be expected to use the standard javac. Etc.

> or should I try to create the workaround?

If possible, or just stay with an annotation declaration which does not trigger the bug to begin with.

Guys, thanks for your interest, I need your help. Pavel created a branch in ergonomics repository called annotation-based-csl-169991. He did not manage to finish his work, but as far as I know, he is able to generate most of the original Ant layer entries. Probably the biggest problem is that he generates slightly more.

Otherwise the issue is solvable. The limitations with JavaC compiler were fixed with few "catch", so we are not limited with JavaC bugs. The branch works with every JDK6 compiler.

My attempt is to finish the originally planned work of performance team for 6.9. This enhancement for 6.9 is still on my radar, but I will happily welcome any help you want to provide. If anyone of you wants to finish this branch, I'll be more than thankful.

Actually, I hit the javac bug in a different case (default severity of a hint, i.e. the default value of the attribute is an enum constant), for which I do not know any reasonable workaround. I am sorry, but it is very likely I could help with this issue except possibly with the workaround for the javac bug.

(In reply to comment #9)
> I need your help.

Meaning me, or who?

Created attachment 93357 [details]
Bundle with Pavel's work done so far

Any help is welcomed. If I get none, I'll see what I can do on Jan 25, 2010.

As a clear sign of weak mind I agreed to have a look at the patch and finish this work. I don't know much about annotation processors, but hopefully most of this stuff was done by Pavel and I'll just have to clean up the resulting layer registrations, which should not be that hard.

Oh well, I had a look at the patch and I can't quite guess what exactly it was going to solve. So I went back to this issue to see what the problems/requirements are and don't seem to be able to guess that either. See below...

(In reply to comment #0)
> Please rewrite the CslJar to @CslRegistration, delete most of the support code
> in CslJar.java and replace it with 
> LayerGeneratingProcessor as advocated at
> http://wiki.apidesign.org/wiki/CompileTimeCache

I understand this.


> 
> The goals:
> 1. move to standard infrastructure
What is the standard infrastructure? Is it LayerGeneratingProcessor? If so, then this is just restating the above.


> 2. delete a lot of useless code
What code? The CslJar task? It's not useless at the moment. But can replaced with an annotation processor, so again probably restating the initial requirement.


> 3. simplify builds scripts (no need for special calls to special ant task)
Ok, this is the same as the initial requirement above.


> 4. improve performance (by registering a proxy with display name, list of mime
> types, etc.)
What exactly is meant by this? A proxy to what? What performance problems are we going to solve? Where are measurements demonstrating these problems?

As far as I can tell Pavel's patch can't work and the direction in which it seems to be going is dubious to say the least. The only requirement that I can see at the moment is to get rid of CslJar, which is legitimate and which I'm going to do in 6.9. IMO it can be done quite easily with much simpler annotation and with almost no impact on the language plugins.

Thanks for looking at Pavel's work, Víťo.

Yes, getting rid of CslJar is necessary initial step (it does not make much sense to improve CslJar task itself, better to fix new LayerGeneratingProcessor). That shall solve #1 (yes, layer generating processor is the standard infrastructure), #2 (you don't need the CslJar's XML manipulation code, keep just the "business logic" rebuilt on top of layer generating processor) and #3 (plus people that depend on CSL will be able to use incremental binary builds http://wiki.netbeans.org/AutoUpdateTask, which are broken right now).

As the #4 goal goes: We want to improve the registrations in layer to prevent classes of unused languages to be loaded. Pavel's changeset
http://hg.netbeans.org/main/rev/270d0d7fca12
shows the list of classes that inspired us to report this issue. The list is growing. The current set of additional violations is produced after every build of http://deadlock.netbeans.org/hudson/job/ergonomics/
and can be found at
http://deadlock.netbeans.org/hudson/job/ergonomics/lastSuccessfulBuild/artifact/ide.kit/build/test/qa-functional/work/o.n.t.i.W/testWhitelist3/whitelist_violators_3.txt
We hope that the @CslRegistration will allow us to make the violation list smaller and identify additional steps ultimately leading to complete elimination of loading CSL related classes of unused languages.

The CslJar task is now history:
http://hg.netbeans.org/jet-main/rev/66c711b525f0
http://hg.netbeans.org/jet-main/rev/7a0adb1bb800

I'm leaving this open in order to lesser the number of loaded classes as requested by Jarda. I doubt I will be able to get rid of all of them, but the situation should get better when Parsing API provides an annotation for registering PathRecognizers.

http://hg.netbeans.org/jet-main/rev/56bb0b6e32b3

Integrated into 'main-golden', will be available in build *201001240200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/66c711b525f0
User: Vita Stejskal <vstejskal@netbeans.org>
Log: #169991: Adding @LanguageRegistration as a replacement for CslJar ant task; converting trunk build modules to use it

Created attachment 93605 [details]
testWhitelist3 violators diff

The diff in the testWhitelist3 violators list after adding @PathRecognizerRegistration and cleaning up MimeLookup registrations.

Created attachment 93606 [details]
@PathRecognizerRegistration annotation added to parsing.api

This patch contains the API change in parsing.api module, which adds @PathRecognizerRegistration annotation. It also contains changes in language plugins, which use this new annotation.

Created attachment 93607 [details]
Additional CSL API cleanup

This patch contains changes in CSL API, which hides implementation details exposed in o.n.m.csl.core and o.n.m.csl.editor packages. The only classes in these packages that are needed by language plugins were several editor actions, which I moved to o.n.m.csl.api. This change plus cleaning up MimeLookup registrations in the previous patch also helped to decrease the number of classes loaded during the whitelist test.

The language plugins dependencies were updated accordingly and their spec versions were incremented in order to propagate this change through AUCs. I plan to do the same for contrib modules.

Please review the API change in parsing.api, which adds new @PathRecognizerRegistration annotation. 

There is also an incompatible change in csl.api, which hides CSL's implementation accidentally exposed in o.n.m.csl.code and o.n.m.csl.editor packages. I'll update csl.api's apichanges.xml and the contrib modules before final push.

Thanks

Created attachment 93693 [details]
Patch for contrib modules

... will be applied when the jet-main changes propagate.

Thanks for the review.

http://hg.netbeans.org/jet-main/rev/a669a9335de4
http://hg.netbeans.org/jet-main/rev/e2805e14bfc0

http://hg.netbeans.org/jet-main/rev/fa629ae8c625

http://hg.netbeans.org/jet-main/rev/8cf7238b12f3

http://hg.netbeans.org/jet-main/rev/a749d8abf225

Integrated into 'main-golden', will be available in build *201002040200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/a749d8abf225
User: Vita Stejskal <vstejskal@netbeans.org>
Log: #169991: incrementing spec versions to fix the VerifyUpdateCenter.diachronicConsistency test failure after e2805e14bfc0