Use -sourcepath to improve compilation speed
#99
Conversation
There are multiple problems with the previous approach of
scanning the current project and including each and every
file to linter javac.
* It takes a long time since every source java file in
the current project is recompiled with every save.
* The command line may become "too long" (see AtomLinter#38)
* It consumes a lot of computer resources (CPU, memory etc)
These problems increase with the number of source files.
Using sourcepath will let javac find the required sourcefiles
by itself, greatly reducing the number of source files it needs
to recompile. This is one of the things we need to make in order
to resolve AtomLinter#98.
It is still not optimal. Javac does not include any caching of the
files it compiles, so saving the same file twice (without editing it)
will recompile it twice as well as any java file referenced by it.
In this commit I also:
* Added ordering to the config variables (I really wanted
the sourcepath option to be next to the classpath option).
* Moved all `if (@verboseLogging)` to the `@_log` method
* Some spacing and cleanup
|
The code LGTM at least, I'll leave it to @florianb to merge though as I'm not familiar with the inner workings of whether it should be changed to this. |
|
👍 My original quote in this PR is not a 100% accurate. |
|
@florianb What do you think of this? |
|
@noseglid thank you very much - this saved me some time! I was indisposed the last few days, so this is the first day i was able to take a look into this PR. If you don't mind i'll leave some inline-comments to discuss some specific points. I am in general fine with the changes and i would really aprecieate to see a test verifying that the sourcepath-option is set as intended. (You may wonder why the package is that poorly tested - since i added the tests later, i decided to not write tests for cases i don't face but to back every ongoing change by a test until i rewrite the whole thing) |
|
Absolutely. I'll be happy to answer any comments you might have. This PR changes current behavior if you don't have your sources in |
| if @verboseLogging | ||
| @_log 'starting to lint.' | ||
| # Sourcepath - this is a fair default for many Java projects | ||
| sp = 'src/main/java/' |
There was a problem hiding this comment.
I really dislike hardcoded defaults. How would it be if we check if the config-setting it references to is a file or a directory?
There was a problem hiding this comment.
I'm not sure I follow you here. This will only have effect if no sourcepath filename (as specified by the config) is found. I set it to this because most java projects (maven or gradle at least) have this sourcepath.
I put this in an effort to keep as much backwards compatability as possible. It'll be more seamless for current users.
There was a problem hiding this comment.
I agree to that, i guess i just didn't express well. I would prefer to put src/main/java/ into the config-setting as default. This would avoid that the user might think no default is set (since i guess some people just skip the fine-prints in the settings-section).
There was a problem hiding this comment.
I'm not sure how to achieve that in an understandable way. The content of the sourcepath file is not in the config (and src/main/java is indeed the content of that file).
Are you suggesting we have one config where the sourcepath file is, and one for its content? Which one should have precedence? How would we convey that?
There was a problem hiding this comment.
I would propose you may either specify a String referencing to directories (f.e. src/main/java or src/main/java:something/else) or referencing to a file (.sourcepath) containing the sourcepath-options.
So if we roll it out, the user might want to check its settings an sees the default "src/main/java", and she might change the setting to a file containing the sourcepath-options, adding another default-directory or managing the setting with the project-mange et al.
|
Thanks again @noseglid -- i really appreciate your collaboration (as well as @Arcanemagus's). I wonder if i shouldn't schedule the rewrite, putting your changes in there. It would be easier to sell the changes, wouldn't it? |
|
The rewrite would be easier with these changes (diffstat: |
|
Sorry for 🐌 the PR - i want to canalize the progress toward the lint-on-the-fly again. 💭 |
|
I guess i can add this treatment of config-values later on.. (in #76) |
|
Looks like the consensus here is that this should be a major version, are there other things waiting or should I push out a release? |
|
@Arcanemagus - huh.. i really hadn't had the time to do the rewrite yet (since i am doing less Java programming for a while). I guess releasing a new major version would be the best. I would then target the rewrite for the next major version. @noseglid - i am really sorry for the latency. This is my bad. |
|
It's okay. I'm using a local copy of this package to use the sourcepath feature. Take your time and do it right. |
There are multiple problems with the previous approach of
scanning the current project and including each and every
file to linter javac.
the current project is recompiled with every save.
These problems increase with the number of source files.
Using sourcepath will let javac find the required sourcefiles
by itself, greatly reducing the number of source files it needs
to recompile. This is one of the things we need to make in order
to resolve #98.
It is still not optimal. Javac does not include any caching of the
files it compiles, so saving the same file twice (without editing it)
will recompile it twice as well as any java file referenced by it.
In this commit I also:
the sourcepath option to be next to the classpath option).
if (@verboseLogging)to the@_logmethod