Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

@Annotations(@Annotation("shared")) is amazingly wasteful #2035

Closed
CeylonMigrationBot opened this issue Feb 2, 2015 · 33 comments
Closed

@Annotations(@Annotation("shared")) is amazingly wasteful #2035

CeylonMigrationBot opened this issue Feb 2, 2015 · 33 comments

Comments

@CeylonMigrationBot
Copy link

[@gavinking] Check this out:

@.com.redhat.ceylon.compiler.java.metadata.Ceylon(major = 8)
@.ceylon.language.DocAnnotation$annotation$(description = "Hello world")
@.ceylon.language.AuthorsAnnotation$annotation$(authors = "Gavin")
@.com.redhat.ceylon.compiler.java.metadata.Annotations({
        @.com.redhat.ceylon.compiler.java.metadata.Annotation(
                value = "doc",
                arguments = {"Hello world"}),
        @.com.redhat.ceylon.compiler.java.metadata.Annotation(
                value = "by",
                arguments = {"Gavin"})})
interface Stuff {}

Duplicate information is stored in a very wasteful format for no apparent reason. I propose that:

  1. we stop storing doc annotation and user-defined annotations in @Annotations, and
  2. we encode language annotations into @annotations using a modifiers integer.

This could be done without breaking backwards compatibility.

[Migrated from ceylon/ceylon-compiler#2035]
[Closed at 2015-02-04 14:33:04]

@CeylonMigrationBot
Copy link
Author

[@tombentley] Early indications are this will make the dist .zip about 6% smaller if we're using a long to encode the modifiers.

@CeylonMigrationBot
Copy link
Author

[@gavinking] Seems worthwhile to me.

Sent from my iPhone

On Feb 3, 2015, at 12:45 PM, Tom Bentley notifications@github.com wrote:

Early indications are this will make the dist .zip about 6% smaller if we're using a long to encode the modifiers.


Reply to this email directly or view it on GitHub.

@CeylonMigrationBot
Copy link
Author

[@tombentley] Sorry, I was completely wrong because I was looking at size of the wrong files. Whether we use an int or a long we're looking at the language module .car being less than 1% smaller.

@CeylonMigrationBot
Copy link
Author

[@quintesse] Doesn't surprise me, strings are only stored once if they're exactly the same. It just looks wasteful in the (fake) generated Java source.

@CeylonMigrationBot
Copy link
Author

[@FroMage] Well, why again didn't we remove the @Annotations one? I think it predates support for annotations, no?

@CeylonMigrationBot
Copy link
Author

[@tombentley] IIRC because the model loader can't use the @Shared$annotation and friends because we can't put those on native things in the language module. In turn because of the bootstrap problem.

@CeylonMigrationBot
Copy link
Author

[@FroMage] Mmm, why not? We are able to use the metamodel defined in Ceylon in its Java implementation, though.

@CeylonMigrationBot
Copy link
Author

[@tombentley] Because in order to do annotation processing javac completes annotations very early on, before the model loader is involved in anything.

@CeylonMigrationBot
Copy link
Author

[@FroMage] Oh, so it's different for annotations? Well, we could still stop generating @Annotations if we're not bootstrapping ;)

@CeylonMigrationBot
Copy link
Author

[@tombentley]

Oh, so it's different for annotations?

yes

Well, we could still stop generating @Annotations if we're not bootstrapping ;)

No the problem is using stuff like @Shared$annotation doesn't exist for javac when we're compiling natives, so we get a no such symbol. javac has no way to know that the symbol will be defined, because we only enter it into the symbol table during a later phase. I think I described the pb wrongly before, but it's been a while since I looked at it. But I do remember it's a pretty hard one to solve.

@CeylonMigrationBot
Copy link
Author

[@FroMage] I don't get it. If we're not compiling the language module, then @Shared$annotation must be resolvable.

@CeylonMigrationBot
Copy link
Author

[@tombentley] Well it was probably about 12 months ago that I was looking at this, so I could be confused, but the problem is what #5286 is about. When we're compiling the natives in the language module javac tries to complete annotation types like @Shared$annotation before our compiler get's a look-in.

@CeylonMigrationBot
Copy link
Author

[@FroMage] Again, I can believe you when bootstrapping, but that should not be true for any other module.

@CeylonMigrationBot
Copy link
Author

[@tombentley] Yes, it's during bootstrapping.

@CeylonMigrationBot
Copy link
Author

[@FroMage] So we should be able to not generate these annotations when not bootstrapping then :)

@CeylonMigrationBot
Copy link
Author

[@tombentley] Well obviously I don't understand how the hell the bootstrapping stuff works, but if that's all it needs... let's have a try.

@CeylonMigrationBot
Copy link
Author

[@quintesse] Meaning we keep the code that knows how to read both types of annotations but we generate the older type only when compiling the language module, right?

@CeylonMigrationBot
Copy link
Author

[@FroMage] Yes, that would be a good start.

@CeylonMigrationBot
Copy link
Author

[@tombentley] Well, sure we could annotate all the native stuff with @Annotations(modifiers=243) (by hand) if you like. But that would be a lot of effort and it still wouldn't fix #5286 because the annotations are read at runtime using the "real" @Shared$annotation annotation. Of course we could also change the way we read language module annotations at runtime, so everything was consistent.

Or we could try to fix the real problem.

@CeylonMigrationBot
Copy link
Author

[@FroMage]

Or we could try to fix the real problem.

Well, we could, but starting by removing @Annotations from every module but the language module. That's easy. The rest is harder as you've pointed out, and IMO can wait.

@CeylonMigrationBot
Copy link
Author

[@quintesse] On the other hand, if the net effect on output size is so little why bother? We could wait until we can do it properly and at least get rid of some code?

@CeylonMigrationBot
Copy link
Author

[@gavinking] Because it is ugly when I turn on logging to see the generated Java schema and have my eyes hurt like someone shoved a stick in them.

That's reason enough.

Sent from my iPhone

On Feb 3, 2015, at 5:36 PM, Tako Schotanus notifications@github.com wrote:

On the other hand, if the net effect on output size is so little why bother? We could wait until we can do it properly and at least get rid of some code?


Reply to this email directly or view it on GitHub.

@CeylonMigrationBot
Copy link
Author

[@quintesse] Go see an optician :)

@CeylonMigrationBot
Copy link
Author

[@tombentley] More fiddly than I would have liked, but hopefully Gavin's eyesight is safe for the time-being.

@CeylonMigrationBot
Copy link
Author

[@gavinking] Thanks! :)

@CeylonMigrationBot
Copy link
Author

[@FroMage] So we didn't remove the $Shared$annotation when we have the info in @Annotation(modifiers = x)? Why not?

@CeylonMigrationBot
Copy link
Author

[@gavinking] We need the $Shared$annotation for the metamodel, don't we?

@CeylonMigrationBot
Copy link
Author

[@FroMage] Also, I think we're better off with the modifier masks somewhere in the language module's metadata package so that they can be defined manually in Java code without pulling the compiler module.

@CeylonMigrationBot
Copy link
Author

[@FroMage]

We need the $Shared$annotation for the metamodel, don't we?

Well, only if the metamodel can't produce it out of thin-air when required.

@CeylonMigrationBot
Copy link
Author

[@tombentley]

So we didn't remove the $Shared$annotation when we have the info in @Annotation(modifiers = x)? Why not?

That's what #5286 is about, and it would involve a similar duplication of code paths in the metamodel. Plus duplicated testing. That's quite a lot more work than this was.

Also, I think we're better off with the modifier masks somewhere in the language module's metadata package so that they can be defined manually in Java code without pulling the compiler module.

I can't really parse that sentence, but I don't have an opposition to putting the masks somewhere else if they need to be shared. Which they would if the metamodel was to read them at runtime.

@CeylonMigrationBot
Copy link
Author

[@FroMage] I meant that if we have to write native code:

@Annotations(modifiers = CeylonModifier.SHARED | CeylonModifier.ABSTRACT)

then it's better if the constants live in the language module's metadata package rather than in the compiler package.

@CeylonMigrationBot
Copy link
Author

[@FroMage]

rather than

Or in addition to if we can't make it work.

@CeylonMigrationBot
Copy link
Author

[@tombentley] Yes, in addition to because the flags being defined in ceylon.language doesn't help because that's not on the classpath of the compiler.

@CeylonMigrationBot CeylonMigrationBot added this to the 1.2 milestone Nov 14, 2015
quintesse pushed a commit that referenced this issue Nov 14, 2015
quintesse pushed a commit that referenced this issue Nov 14, 2015
…nstead of nested @annotation for the modifier annotations. Omit documentation annotations completely.
quintesse pushed a commit that referenced this issue Nov 14, 2015
quintesse pushed a commit that referenced this issue Nov 14, 2015
quintesse pushed a commit that referenced this issue Nov 14, 2015
…t now handles documentation annotations too.
quintesse pushed a commit that referenced this issue Nov 14, 2015
quintesse pushed a commit that referenced this issue Nov 14, 2015
… annotations with both styles of annotation: @annotations and @shared$annotation$
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants