-
-
Notifications
You must be signed in to change notification settings - Fork 351
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
REVIEW add support for incremental build #1905
Conversation
@@ -75,7 +75,7 @@ | |||
|
|||
public static final String OUTPUTDIR = "spooned"; | |||
|
|||
private final Factory factory; | |||
protected Factory factory; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really necessary to make the factory protected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can replace that with super(oldFactory)
, but I would like to avoid undesirable call of processArguments()
in the Launcher
constructor. Alternatively, I can add another protected constructor to the Launcher
, which does not call processArguments()
.
I'm not really sure why you created a new Launcher for that feature: why not creating a class managing the serialization that you can use in the current Launcher if a specific option is given? And maybe adding a new property in environment to manage the cache directory. My point is your new |
Before adding new code, could you, in a first PR, remove the code related to the old and broken 'build-outdated' feature? Thanks! |
@surli Ok, so what API do you suggest?
I think, I can refactor it this way. |
Actually you could even put the And isn't the cache directory mandatory in case of incremental build? So I suggest this kind of API then: Launcher launcher = new Launcher();
launcher.addInputResources(....);
launcher.getEnvironment.setClasspath(....);
launcher.getEnvironment().setIncremental(true);
launcher.getEnvironment().setCacheDir(....); // optional: a default value should be available
launcher.build(); WDYT? |
Ok. I'll try to implement this API. |
Any news on this one @Egor18 ? |
@surli I'll do that as soon, as I can. |
Any news? I am very interested in this feature, it looks quite related to #1983 Would be happy to help Cheers, |
just restarted Travis |
Thanks a lot for the contribution! |
Moved from #1816
Hi, this is my new implementation of the incremental build.
A few words about performance.
On my machine spoon model for RxJava project builds in about 15 minutes, but I can get cached model in less than 1 minute. So, on some projects cache itself could be quite useful.
I added
BufferedOutputStream
andBufferedInputStream
toSerializationModelStreamer
because they drastically increase model serialization/deserialization speed (dozens of times or even more).However, it's possible that reference resolution part for incremental build could be rewritten in a more effective way (as I mentioned here).
So what do you think about this incremental launcher?