Initialize CDI observers properly #489
Conversation
mkouba
commented
Jan 13, 2019
mkouba
commented
- resolves NullPointerException when resolving a CDI injection #477
- resolves #477
| for (InterceptorInfo interceptor : interceptors) { | ||
| interceptor.init(); | ||
| } | ||
| beans.forEach(BeanInfo::init); |
There was a problem hiding this comment.
I don't know if we want to be encouraging the use of lambda's like this. At the moment AFAIK they use about 4k of memory in metaspace, so this code will now use 12k of metaspace instead of basically being free. In this case it is in the processor so I don't know how much it matters as it is not runtime.
@n1hility @dmlloyd do you think we should ban the use of lambas in the code base? Even though they make some code a bit cleaner I don't think it is worth the memory overhead.
There was a problem hiding this comment.
Well, we already use a lot of lambdas in processors and I think that it's probably worth the memory overhead. Also we should have some exact numbers - is it really 4K for a method reference? I tried to find some relevant info but I did not find anything.
There was a problem hiding this comment.
This info may be old, at one point the JDK was basically creating a new class loader per lambda, and that class loader allocates 4kb of space by default. It sounds like this was something that could be fixed, however I am not sure if it has been.
There was a problem hiding this comment.
FWIW I've been using lambdas with a fairly clear conscience in code that is run at augment time only, or code that will only ever be used in substrate mode. I've only been avoiding them for runtime code that can be run in JVM mode.
|
Other than the lambda issue I think this looks good to me. |
|
I am just going to merge this for now, if we decide to do something about reducing lambdas in the code base it will need to be addressed on a project level. |
today it only check the //SOURCE, but it must also consider the origin. [patch]
