You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2020/01/21 06:27:56 UTC
[GitHub] [netbeans] Akshay-Gupta-Oracle opened a new pull request #1890:
[NETBEANS-3693] Create only one instance of javac
Akshay-Gupta-Oracle opened a new pull request #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890
[NETBEANS-3693] Create only one instance of javac for parsing multiple files when refactored
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] Akshay-Gupta-Oracle edited a comment on issue #1890:
[NETBEANS-3693] Create only one instance of javac
Posted by GitBox <gi...@apache.org>.
Akshay-Gupta-Oracle edited a comment on issue #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#issuecomment-598071283
@jlahoda
Does the issue in this PR(Making multiple instances of javac for parsing multiple files) is addressed in this other PR #2013 ??
Or should I keep this one open??
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lkishalmi commented on issue #1890: [NETBEANS-3693]
Create only one instance of javac
Posted by GitBox <gi...@apache.org>.
lkishalmi commented on issue #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#issuecomment-592059791
@jlahoda or @lahodaj as the 12.0 merge window is open, can you review the changes?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] Akshay-Gupta-Oracle commented on issue #1890:
[NETBEANS-3693] Create only one instance of javac
Posted by GitBox <gi...@apache.org>.
Akshay-Gupta-Oracle commented on issue #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#issuecomment-584013441
> > > Left some initial comments. Please note that at least the "org.netbeans.modules.java.source.parsing.JavacParserTest" appears to be failing.
> >
> >
> > There is one test case failing - testMultiSourceVanilla
> > Which has the statement - "assertFalse(Objects.equals(storedJLObject, jlObject));" which I think will not pass when only one instance of javac is created.
>
> Right - but that means the test needs to be fixed (or removed), correct? I.e. we cannot have failing tests, I assume?
Yes, will do that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jlahoda commented on issue #1890: [NETBEANS-3693] Create
only one instance of javac
Posted by GitBox <gi...@apache.org>.
jlahoda commented on issue #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#issuecomment-593252885
> @jlahoda or @lahodaj as the 12.0 merge window is open, can you review the changes?
Currently, without nb-javac, refactoring run file-by-file (one javac instance per file), which increases chances it fits into the memory. This patch changes that to one javac instance for all files in the refactoring - that significantly improves the speed, but lowers the chances the processing will fit into the memory. I still think a middle ground - try to fit all into the memory, and run fast if it fits, but fallback to file-by-file processing if it doesn't - is the best combination. And, unless I am mistaken, this patch does not provide that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lahodaj commented on a change in pull request #1890:
[NETBEANS-3693] Create only one instance of javac
Posted by GitBox <gi...@apache.org>.
lahodaj commented on a change in pull request #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#discussion_r369128971
##########
File path: java/java.source.base/src/org/netbeans/modules/java/source/parsing/JavacParser.java
##########
@@ -181,8 +182,8 @@
private FileObject root;
//ClassPaths used by the parser
private ClasspathInfo cpInfo;
- //Count of files the parser was created for
- private final int sourceCount;
+ //XXX: Count of files the parser was created for
Review comment:
Would probably be good to cleanup the comment here (i.e. adjust it to the current state).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lahodaj commented on issue #1890: [NETBEANS-3693] Create
only one instance of javac
Posted by GitBox <gi...@apache.org>.
lahodaj commented on issue #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#issuecomment-584012522
> > Left some initial comments. Please note that at least the "org.netbeans.modules.java.source.parsing.JavacParserTest" appears to be failing.
>
> There is one test case failing - testMultiSourceVanilla
> Which has the statement - "assertFalse(Objects.equals(storedJLObject, jlObject));" which I think will not pass when only one instance of javac is created.
Right - but that means the test needs to be fixed (or removed), correct? I.e. we cannot have failing tests, I assume?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] Akshay-Gupta-Oracle commented on issue #1890:
[NETBEANS-3693] Create only one instance of javac
Posted by GitBox <gi...@apache.org>.
Akshay-Gupta-Oracle commented on issue #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#issuecomment-598071283
@jlahoda
Does the issue in this PR(Making multiple instances of javac for parsing multiple files) is addressed in this other PR #2013 ??
Or should I keep it open??
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] Akshay-Gupta-Oracle commented on issue #1890:
[NETBEANS-3693] Create only one instance of javac
Posted by GitBox <gi...@apache.org>.
Akshay-Gupta-Oracle commented on issue #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#issuecomment-583981028
> Left some initial comments. Please note that at least the "org.netbeans.modules.java.source.parsing.JavacParserTest" appears to be failing.
There is one test case failing - testMultiSourceVanilla
Which has the statement - "assertFalse(Objects.equals(storedJLObject, jlObject));" which I think will not pass when only one instance of javac is created.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lahodaj commented on a change in pull request #1890:
[NETBEANS-3693] Create only one instance of javac
Posted by GitBox <gi...@apache.org>.
lahodaj commented on a change in pull request #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#discussion_r369128641
##########
File path: java/java.source.base/src/org/netbeans/modules/java/source/parsing/JavacParser.java
##########
@@ -415,9 +422,28 @@ private void parseImpl(
break;
default:
init (snapshot, task, false);
+ DiagnosticListener<JavaFileObject> diagnosticListener;
Review comment:
I was hoping this could work in such a way that if the processing of the files does not fit into memory, it would fall back to the slow "one javac per file". It is not as good as current nb-javac, but I hope it would work reasonably OK. Would also be good to have some tests for that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lahodaj commented on a change in pull request #1890:
[NETBEANS-3693] Create only one instance of javac
Posted by GitBox <gi...@apache.org>.
lahodaj commented on a change in pull request #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#discussion_r369104253
##########
File path: java/java.source.base/src/org/netbeans/modules/java/source/parsing/JavacParser.java
##########
@@ -209,11 +210,16 @@
private ChangeListener weakCpListener;
//Current source for parse optmalization of task with no Source (identity)
private Reference<JavaSource> currentSource;
+ //cache of already parsed files
+ static private Map<String, CompilationUnitTree> parsedTrees;
Review comment:
Declaring a static map generally provokes various questions (like when is the map cleared, what is the threading (although that one is much smaller issue here), etc.). In general, it is better to avoid static maps like this. I would suggest to try to move the map into the CompilationInfoImpl (as an instance field) - then its lifecycle would be much clearer.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lahodaj commented on a change in pull request #1890:
[NETBEANS-3693] Create only one instance of javac
Posted by GitBox <gi...@apache.org>.
lahodaj commented on a change in pull request #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#discussion_r369121184
##########
File path: java/java.source.base/src/org/netbeans/modules/java/source/parsing/JavacParser.java
##########
@@ -582,24 +608,37 @@ Phase moveToPhase (final Phase phase, final CompilationInfoImpl currentInfo,
}
long start = System.currentTimeMillis();
// XXX - this might be with wrong encoding
- Iterable<? extends CompilationUnitTree> trees;
+ Iterable<? extends CompilationUnitTree> trees=null;
Review comment:
This is a sort-of a nit comment, but please keep formatting similar to the rest of the file and codebase - we typically use spaces around '=', spaces between 'if' and '(', say "} else {" on a single line, etc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lahodaj commented on a change in pull request #1890:
[NETBEANS-3693] Create only one instance of javac
Posted by GitBox <gi...@apache.org>.
lahodaj commented on a change in pull request #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#discussion_r369129150
##########
File path: java/java.source.base/src/org/netbeans/modules/java/source/parsing/JavacParser.java
##########
@@ -582,24 +608,37 @@ Phase moveToPhase (final Phase phase, final CompilationInfoImpl currentInfo,
}
long start = System.currentTimeMillis();
// XXX - this might be with wrong encoding
- Iterable<? extends CompilationUnitTree> trees;
+ Iterable<? extends CompilationUnitTree> trees=null;
+ CompilationUnitTree unit=null;
+ if(snapshots.size()>1 && parsedTrees.containsKey(currentInfo.jfo.getName())){
+ unit=parsedTrees.get(currentInfo.jfo.getName());
+ }
+ else{
if (sequentialParsing != null) {
trees = sequentialParsing.parse(currentInfo.getJavacTask(), currentInfo.jfo);
} else {
trees = currentInfo.getJavacTask().parse();
}
- if (trees == null) {
- LOGGER.log( Level.INFO, "Did not parse anything for: {0}", currentInfo.jfo.toUri()); //NOI18N
- return Phase.MODIFIED;
- }
- Iterator<? extends CompilationUnitTree> it = trees.iterator();
- if (!it.hasNext()) {
- LOGGER.log( Level.INFO, "Did not parse anything for: {0}", currentInfo.jfo.toUri()); //NOI18N
- return Phase.MODIFIED;
+ if (unit == null) {
+ if (trees == null) {
+ LOGGER.log(Level.INFO, "Did not parse anything for: {0}", currentInfo.jfo.toUri()); //NOI18N
+ return Phase.MODIFIED;
+ }
+ Iterator<? extends CompilationUnitTree> it = trees.iterator();
+ if (!it.hasNext()) {
+ LOGGER.log(Level.INFO, "Did not parse anything for: {0}", currentInfo.jfo.toUri()); //NOI18N
+ return Phase.MODIFIED;
+ }
+ while (it.hasNext()) {
+ CompilationUnitTree oneFileTree = it.next();
+ CompilationUnitTree put = parsedTrees.put( oneFileTree.getSourceFile().getName(), oneFileTree);
+ }
+ unit = trees.iterator().next();
+ }
}
- CompilationUnitTree unit = it.next();
+
currentInfo.setCompilationUnit(unit);
- assert !it.hasNext();
+ //assert !it.hasNext();
Review comment:
Probably just delete the assert.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jlahoda commented on issue #1890: [NETBEANS-3693] Create
only one instance of javac
Posted by GitBox <gi...@apache.org>.
jlahoda commented on issue #1890: [NETBEANS-3693] Create only one instance of javac
URL: https://github.com/apache/netbeans/pull/1890#issuecomment-598577091
> @jlahoda
> Does the issue in this PR(Making multiple instances of javac for parsing multiple files) is addressed in this other PR #2013 ??
> Or should I keep this one open??
I don't think there is a significant overlap between these PRs - this PR is (as far as I can tell) speeding up refactoring. #2013 is fixing the refactoring.java tests+adding them into the Travis configuration.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists