ArC - add "remove unused beans" optimization #552
Conversation
| @@ -70,4 +72,9 @@ BeanContainerListenerBuildItem build( | |||
|
|
|||
| } | |||
|
|
|||
| @BuildStep | |||
| public BeanRemovalExclusionBuildItem removalExclusion() { | |||
There was a problem hiding this comment.
I don't really like the name of this build item. I also don't think it should be necessary for beans added through AdditionalBeanBuildItem.
I think something like UsedBeanBuildItem would be better, the current one feels a bit like a double negative.
There was a problem hiding this comment.
It is necessary for AdditionalBeanBuildItem - extensions do add many additional beans but these beans are not always needed at runtime.
UsedBeanBuildItem does not sound quite right to me. Hm, maybe something like UnremovableBeanBuildItem?
There was a problem hiding this comment.
Take the ConfigProducer as an examle of AdditionalBeanBuildItem that could be removed.
There was a problem hiding this comment.
Maybe AdditionalBeanBuildItem should have a 'removable' parameter?
There was a problem hiding this comment.
That's exactly what I'm working on right now ;-)
|
|
||
| @BuildStep | ||
| public BeanRemovalExclusionBuildItem removalExclusion() { | ||
| return new BeanRemovalExclusionBuildItem(new BeanClassAnnotationExclusion("javax.ws.rs")); |
There was a problem hiding this comment.
I also don't think this should be necessary, IMHO if a bean is added via an annotation specified in BeanDefiningAnnotationBuildItem then I don't think we should be removing it.
There was a problem hiding this comment.
Hm, this case is a little bit different and I agree that we should probably register UnremovableBeanBuildItem for BeanDefiningAnnotationBuildItems automatically.
- ported from Weld - it is enabled by default - resolves #540
mkouba
force-pushed
the
issue-540-removeunusedbeans-next
branch
from
986b103 to
e5720eb
Compare
|
@stuartwdouglas Both
I've also rebased the branch. |
| /** | ||
| * This build item is used to specify additional bean classes to be analyzed. | ||
| * <p> | ||
| * By default, the resulting beans should be removed if they're considered unused as described in {@link ArcConfig#removeUnusedBeans}. |
There was a problem hiding this comment.
Minor issue, but 'should be removed' is confusing, it should say that they are eligible for removal if they are considered unused and removal is enabled
There was a problem hiding this comment.
'may be removed' would be better
There was a problem hiding this comment.
Ok. You know my english is cumbersome ;-)
mkouba
force-pushed
the
issue-540-removeunusedbeans-next
branch
from
ef2bfdb to
2afc248
Compare
|
@stuartwdouglas Anything else to add/improve? |
|
Time is up I'm going to merge this ;-) Feel free create new issues if you find something important... |
So this is basically a framework-level dead code elimination. The container attempts to remove all unused beans before the wiring metadata classes are generated. The algorithm is described in javadoc and is very similar to the Weld implementation.
The optimization is enabled by default, but could be disabled by
shamrock.arc.remove-unused-beans=false.Extensions can register
BeanRemovalExclusionBuildItems to eliminate false positives such as JAX-RS resources (in general application entry points and beans that are only accessed programatically).In terms of build times - for
quickstarts/getting-startedthe difference is:So the build is a bit faster and we generate 22 classes less.