Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for incremental annotation processing in Gradle #482

Closed
harliedharma opened this issue Jan 29, 2019 · 2 comments

Comments

@harliedharma
Copy link

commented Jan 29, 2019

Gradle supports incremental annotation processing since version 4.7 to speed up compilation process.
For details see:
https://docs.gradle.org/nightly/userguide/java_plugin.html#sec:incremental_annotation_processing

gradle/gradle#5277

Litho currently doesn't support incremental annotation processing. AFAIK, by default, it will make Gradle disable incremental annotation processing for the entire module. It will be great if Litho enable this feature as Litho heavily depends on annotation processing.

@pavlospt

This comment has been minimized.

Copy link
Contributor

commented Jan 29, 2019

I can take a look at this and give it a try!

facebook-github-bot added a commit that referenced this issue Feb 23, 2019
Summary:
This PR handles required changes to make Litho compatible with Gradle incremental annotation processing.

* Add originating element when creating a `*SpecModel`.
    * This is required, in order to pass it on the `TypeSpec` that in turn forwards those to `Filer API`.
* Add relevant Gradle META-INF file that specify which annotation processors support incremental AP and what is their type. (I explicitly left out any `*TestingProcessor` in order to avoid any false alarms when running tests)

I did some manual tests and it seems that there is indeed a performance improvement between this branch and `master`, but I am not quite sure if I am biased or this is working in reality. I have some screenshots indicating that less tasks were executed for the same changes between `pavlospt:master` (rebased on latest `litho:master`) and `pavlospt:incremental_annotation_processing`.

![Java changes on `master`](https://user-images.githubusercontent.com/1070871/51906479-dbd54b00-23cc-11e9-8349-63259fcffd6f.png)

![Java changes on `incremental_annotation_processing`](https://user-images.githubusercontent.com/1070871/51906511-eabbfd80-23cc-11e9-8a01-79ec36cb0c83.png)

I am not sure if this can be tested. Other implementations of IAP i have found around, are making use of a `Map` inside the `Processor` and return the `orginatingElement` from an equivalent method there. Essentially they test the existence of an element's FQCN in the `Map`.

The way I implemented the current solution, is just by taking advantage of `TypeSpec` 's ability to hold an `originatingElement` which then passes it to the `Filer API`. I guess we can trust Square's tests for this 😄

Addresses issue #482
Pull Request resolved: #483

Reviewed By: IanChilds

Differential Revision: D13859682

Pulled By: passy

fbshipit-source-id: d4c5eaa1d65385227b259f3e471d9a0467284cc0
@pavlospt

This comment has been minimized.

Copy link
Contributor

commented Feb 23, 2019

@harliedharma the PR for incremental annotation processing has been merged! You can close this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.