Please rewrite the CslJar to @CslRegistration, delete most of the support code in CslJar.java and replace it with
LayerGeneratingProcessor as advocated at
1. move to standard infrastructure
2. delete a lot of useless code
3. simplify builds scripts (no need for special calls to special ant task)
4. improve performance (by registering a proxy with display name, list of mime types, etc.)
Seems pflaska is working on this in
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 bug Pavel talked about is:
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.
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.
Comment 13Vitezslav Stejskal
2010-01-18 09:53:41 UTC
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.
Comment 14Vitezslav Stejskal
2010-01-20 09:53:11 UTC
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
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.
Comment 19Vitezslav Stejskal
2010-01-27 09:26:19 UTC
Created attachment 93605[details]
testWhitelist3 violators diff
The diff in the testWhitelist3 violators list after adding @PathRecognizerRegistration and cleaning up MimeLookup registrations.
Comment 20Vitezslav Stejskal
2010-01-27 09:35:42 UTC
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.
Comment 21Vitezslav Stejskal
2010-01-27 09:41:16 UTC
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.
Comment 22Vitezslav Stejskal
2010-01-27 09:49:46 UTC
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.
Comment 23Vitezslav Stejskal
2010-01-31 16:13:08 UTC