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 2022/05/31 22:51:36 UTC

[GitHub] [netbeans] notzed opened a new pull request, #4180: Improve output window performance for ant java tasks #4141.

notzed opened a new pull request, #4180:
URL: https://github.com/apache/netbeans/pull/4180

   These two together improve a test case of 10000 lines of 2890
   characters from taking an estimated 25 minutes to run to taking 5
   seconds:
    Change to use matches() rather than find() for the stack checking
     regex, which runs very slowly on long lines.
    Use buffered reads rather than single byte reads.
   
   Other changes:
    Use an ExecutorService to start and manage threads.
    Remove references to deprecated ThreadDeath exception.
    Move critical section to isolated functions.
    Split non-processing and processing stream handlers
     into separate classes.
    Fix some logic errors introduced from / and requiring ancient
     'hot fixes'.
   
   
   
   
   
   ---
   **^Add meaningful description above**
   
   By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -
   
    - are all your own work, and you have the right to contribute them.
    - are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).
   
   Please make sure (eg. `git log`) that all commits have a valid name and email address for you in the Author field.
   
   If you're a first time contributor, see the Contributing guidelines for more information.
   
   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#issuecomment-1159355088

   all good. now hands off :)


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] notzed commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
notzed commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r898647962


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                    tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                    tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));

Review Comment:
   Well now this code has to make a decision on what to do - the only exception being handled (illegalargumentexception) happens only if the property is null, and that case would've previously thrown nullpointerexception in String:lookupCharset().  Either way having a fallback is potentially a behavioural change.
   
   I looked at an alternative approach but then noticed ByteArrayOutputStream:toString(Charset) was introduced at Java 10, so it can't be used anyway can it?
   
   For what it's worth I had looked into a different buffering mechanism previously that could just use String:new directly but it didn't seem worth the added code.
   



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r898656630


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                    tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                    tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));

Review Comment:
   > ByteArrayOutputStream:toString(Charset) was introduced at Java 10, so it can't be used anyway can it?
   
   oh wow you are right. I haven't checked for that. Lets forget about this then - my mistake. Lets only fix the `errEncoding` and we are good.
   
   just as sidenote:
   all other exceptions the method throws are subtypes of IllegalArgumentException, so it is basically catching everything, not only the null case (but making this a multi-catch would be clearer probably).



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r889637652


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                        tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                        tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                } else {
+                    setCopier(stdout, getOutputStream(), delegateOutputStream, false);
+                    setCopier(stderr, getErrorStream(), delegateErrorStream, true);
+                }                                

Review Comment:
   you can actually configure it on a per-project basis which would override the global default. But NetBeans has ~840 sub projects so its probably not worth the trouble :)
   
   I think the only change I made from the global default is to set tab size to 4 (I believe 8 is default). But this would only show in files which actually use tabs.
   
   I rarely use auto formatting. If i do its usually only for few selected lines since it creates too much noise in the diff.
   
   It was not super important anyway, I only mentioned it since I noticed it while looking through it.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#issuecomment-1159535953

   @notzed congrats to your first contribution
   
   btw you can link your commit author details to your account by adding the email address you used for the commit to your github account somewhere in the settings.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] notzed commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
notzed commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r887368269


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,228 +166,222 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst().ifPresentOrElse(buildLogger -> {
+                        tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                        tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                    }, ()->{

Review Comment:
   huh, i guess there's another netbeans bug then because the source level is set to 1.8 and it didn't pick this up.
   



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] notzed commented on pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
notzed commented on PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#issuecomment-1144225674

   As I said in the problem report when i first created the fork i tried to fix the username in the commit but couldn't.  I've changed it in that repository and tried adding changes but as soon as i squash the changes it just uses the original patch.  I tried it about 5 different ways which is why i said i would probably have to do it in another branch and just use patch to transfer the changes.
   
   I've gone by notzed for over 25 years as my online nick and use it for all my non-paid and free software development work, so it was definitely an intentional choice to use it for this site.
   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] notzed commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
notzed commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r898594068


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                    tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                    tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));

Review Comment:
   Thanks for spotting the paste-o, i agree on the charset change, i just left it as is to avoid unnecessary changes and since that form is widely used everywhere else in java code and avoids the need to handle the exception.
   
   Do you want me to apply this diff and re-push the pull request?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r889755920


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                        tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                        tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                } else {
+                    setCopier(stdout, getOutputStream(), delegateOutputStream, false);
+                    setCopier(stderr, getErrorStream(), delegateErrorStream, true);
+                }                                
+                InputStream is = getInputStream();
+                if (is == null)
+                    is = AntBridge.delegateInputStream();
+                inputTask = tasks.submit(new TransferCopier(is, stdin));
+            }
 
             public void stop() {
-                if (errTask != null) {
-                    try {
-                        errTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outTask != null) {
-                    try {
-                        outTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (inTask != null) {
-                    inTask.interrupt();
-                    try {
-                        inTask.join(1000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outCopier != null) {
-                    outCopier.maybeFlush();
-                }
-                if (errCopier != null) {
-                    errCopier.maybeFlush();
+                try {
+                    if (inputTask != null)
+                        inputTask.cancel(true);
+                    tasks.shutdown();
+                    tasks.awaitTermination(3, TimeUnit.SECONDS);
+                } catch (InterruptedException ex) {
+                } finally {
+                    tasks.shutdownNow();
                 }
             }
 
             public void setProcessOutputStream(InputStream inputStream) throws IOException {
-                OutputStream os = getOutputStream();
-                Integer logLevel = null;
-                if (os == null || delegateOutputStream) {
-                    os = AntBridge.delegateOutputStream(false);
-                    logLevel = Project.MSG_INFO;
-                }
-                outTask = new Thread(Thread.currentThread().getThreadGroup(), outCopier = new Copier(inputStream, os, logLevel, outEncoding, foldingHelper),
-                        "Out Thread for " + getProject().getName()); // NOI18N
-                outTask.setDaemon(true);
-                outTask.start();
+                this.stdout = inputStream;
             }
 
             public void setProcessErrorStream(InputStream inputStream) throws IOException {
-                OutputStream os = getErrorStream();
-                Integer logLevel = null;
-                if (os == null || delegateErrorStream) {
-                    os = AntBridge.delegateOutputStream(true);
-                    logLevel = Project.MSG_WARN;
-                }
-                errTask = new Thread(Thread.currentThread().getThreadGroup(), errCopier = new Copier(inputStream, os, logLevel, errEncoding, foldingHelper),
-                        "Err Thread for " + getProject().getName()); // NOI18N
-                errTask.setDaemon(true);
-                errTask.start();
+                this.stderr = inputStream;
             }
 
             public void setProcessInputStream(OutputStream outputStream) throws IOException {
-                InputStream is = getInputStream();
-                if (is == null) {
-                    is = AntBridge.delegateInputStream();
-                }
-                inTask = new Thread(Thread.currentThread().getThreadGroup(), new Copier(is, outputStream, null, null, foldingHelper),
-                        "In Thread for " + getProject().getName()); // NOI18N
-                inTask.setDaemon(true);
-                inTask.start();
+                this.stdin = outputStream;
             }
 
         }
 
     }
 
-    private class Copier implements Runnable {
+    /**
+     * Simple copier that transfers all input to output.
+     */
+    private class TransferCopier implements Runnable {
 
         private final InputStream in;
         private final OutputStream out;
-        private final Integer logLevel;
+
+        public TransferCopier(InputStream in, OutputStream out) {
+            this.in = in;
+            this.out = out;
+        }
+
+        @Override
+        public void run() {
+            try {
+                byte[] data = new byte[1024];
+                int len;
+                while ((len = in.read(data)) >= 0) {
+                    out.write(data, 0, len);
+                    out.flush();
+                }
+            } catch (IOException  x) {
+                // ignore IOException: Broken pipe from FileOutputStream.writeBytes in BufferedOutputStream.flush
+            }
+        }
+
+    }
+
+    /**
+     * Filtering copier that marks up links, ignoring stack traces.
+     */
+    private class MarkupCopier implements Runnable {
+
+        private final InputStream in;
+        private final int logLevel;
         private final String encoding;
         private final RequestProcessor.Task flusher;
         private final ByteArrayOutputStream currentLine;
-        private OutputWriter ow = null;
-        private boolean err;
-        private AntSession session = null;
+        private final OutputWriter ow;
+        private final boolean err;
+        private final AntSession session;
         private final FoldingHelper foldingHelper;
 
-        public Copier(InputStream in, OutputStream out, Integer logLevel, String encoding/*, long init*/,
-                FoldingHelper foldingHelper) {
+        public MarkupCopier(InputStream in, int logLevel, boolean err, String encoding, NbBuildLogger buildLogger, FoldingHelper foldingHelper) {
             this.in = in;
-            this.out = out;
             this.logLevel = logLevel;
+            this.err = err;
             this.encoding = encoding;
             this.foldingHelper = foldingHelper;
-            if (logLevel != null) {
-                flusher = PROCESSOR.create(new Runnable() {
-                    public void run() {
-                        maybeFlush();
-                    }
-                });
-                currentLine = new ByteArrayOutputStream();
+
+            flusher = PROCESSOR.create(() -> maybeFlush(false));
+            currentLine = new ByteArrayOutputStream();
+
+            ow = err ? buildLogger.err : buildLogger.out;
+            session = buildLogger.thisSession;
+        }
+
+        private synchronized void append(byte[] data, int off, int len) {
+            currentLine.write(data, off, len);
+            if (currentLine.size() > 8192) {
+                flusher.run();
             } else {
-                flusher = null;
-                currentLine = null;
+                flusher.schedule(250);
             }
         }
 
+        private synchronized String appendAndTake(byte[] data, int off, int len) throws UnsupportedEncodingException {
+            currentLine.write(data, off, len);
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
+        private synchronized String take() throws UnsupportedEncodingException {
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
         public void run() {
-            /*
-            StringBuilder content = new StringBuilder();
-            long tick = System.currentTimeMillis();
-            content.append(String.format("[init: %1.1fsec]", (tick - init) / 1000.0));
-             */
-            
-            if (ow == null && logLevel != null) {
-                Vector v = getProject().getBuildListeners();
-                for (Object o : v) {
-                    if (o instanceof NbBuildLogger) {
-                        NbBuildLogger l = (NbBuildLogger) o;
-                        err = logLevel != Project.MSG_INFO;
-                        ow = err ? l.err : l.out;
-                        session = l.thisSession;
-                        break;
-                    }
-                }
-            }
             try {
+                byte[] data = new byte[1024];
+                int len;
+
                 try {
-                    int c;
-                    while ((c = in.read()) != -1) {
-                        if (logLevel == null) {
-                            // Input gets sent immediately.
-                            out.write(c);
-                            out.flush();
-                        } else {
-                            synchronized (this) {
-                                if (c == '\n') {                                    
-                                    String str = currentLine.toString(encoding);
-                                    int len = str.length();
-                                    if (len > 0 && str.charAt(len - 1) == '\r') {
-                                        str = str.substring(0, len - 1);
-                                    }
+                    while ((len = in.read(data)) >= 0) {
+                        int last = 0;
+                        for (int i = 0; i < len; i++) {
+                            int c = data[i] & 0xff;
 
+                            // Add folds for stack traces and mark up lines
+                            // not processed by JavaAntLogger stack trace detection
+                            if (c == '\n') {
+                                String str = appendAndTake(data, last, i > last && data[i - 1] == '\r' ? i - last - 1 : i - last);
+
+                                synchronized (foldingHelper) {
                                     foldingHelper.checkFolds(str, err, session);
-                                    if (str.length() < LOGGER_MAX_LINE_LENGTH) { // not too long message, probably interesting
-                                        // skip stack traces (hyperlinks are created by JavaAntLogger), everything else write directly
-                                        if (!STACK_TRACE.matcher(str).find()) {
-                                            StandardLogger.findHyperlink(str, session, null).println(session, err);
-                                        }
-                                    } else {
-                                        // do not match long strings, directly create a trivial hyperlink
+                                    if (str.length() >= LOGGER_MAX_LINE_LENGTH || !STACK_TRACE.matcher(str).matches())
                                         StandardLogger.findHyperlink(str, session, null).println(session, err);

Review Comment:
   yes I think you are right. The regex already matches the full line, `find()` would be redundant and likely much slower.
   I mostly tried to find an excuse to use the regex checker window #4200.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] ebarboni commented on pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
ebarboni commented on PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#issuecomment-1143264173

   Hi, maybe you should check your name in your git sound like an avatar


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r889427864


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                        tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                        tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                } else {
+                    setCopier(stdout, getOutputStream(), delegateOutputStream, false);
+                    setCopier(stderr, getErrorStream(), delegateErrorStream, true);
+                }                                
+                InputStream is = getInputStream();
+                if (is == null)
+                    is = AntBridge.delegateInputStream();
+                inputTask = tasks.submit(new TransferCopier(is, stdin));
+            }
 
             public void stop() {
-                if (errTask != null) {
-                    try {
-                        errTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outTask != null) {
-                    try {
-                        outTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (inTask != null) {
-                    inTask.interrupt();
-                    try {
-                        inTask.join(1000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outCopier != null) {
-                    outCopier.maybeFlush();
-                }
-                if (errCopier != null) {
-                    errCopier.maybeFlush();
+                try {
+                    if (inputTask != null)
+                        inputTask.cancel(true);
+                    tasks.shutdown();
+                    tasks.awaitTermination(3, TimeUnit.SECONDS);
+                } catch (InterruptedException ex) {
+                } finally {
+                    tasks.shutdownNow();
                 }
             }
 
             public void setProcessOutputStream(InputStream inputStream) throws IOException {
-                OutputStream os = getOutputStream();
-                Integer logLevel = null;
-                if (os == null || delegateOutputStream) {
-                    os = AntBridge.delegateOutputStream(false);
-                    logLevel = Project.MSG_INFO;
-                }
-                outTask = new Thread(Thread.currentThread().getThreadGroup(), outCopier = new Copier(inputStream, os, logLevel, outEncoding, foldingHelper),
-                        "Out Thread for " + getProject().getName()); // NOI18N
-                outTask.setDaemon(true);
-                outTask.start();
+                this.stdout = inputStream;
             }
 
             public void setProcessErrorStream(InputStream inputStream) throws IOException {
-                OutputStream os = getErrorStream();
-                Integer logLevel = null;
-                if (os == null || delegateErrorStream) {
-                    os = AntBridge.delegateOutputStream(true);
-                    logLevel = Project.MSG_WARN;
-                }
-                errTask = new Thread(Thread.currentThread().getThreadGroup(), errCopier = new Copier(inputStream, os, logLevel, errEncoding, foldingHelper),
-                        "Err Thread for " + getProject().getName()); // NOI18N
-                errTask.setDaemon(true);
-                errTask.start();
+                this.stderr = inputStream;
             }
 
             public void setProcessInputStream(OutputStream outputStream) throws IOException {
-                InputStream is = getInputStream();
-                if (is == null) {
-                    is = AntBridge.delegateInputStream();
-                }
-                inTask = new Thread(Thread.currentThread().getThreadGroup(), new Copier(is, outputStream, null, null, foldingHelper),
-                        "In Thread for " + getProject().getName()); // NOI18N
-                inTask.setDaemon(true);
-                inTask.start();
+                this.stdin = outputStream;
             }
 
         }
 
     }
 
-    private class Copier implements Runnable {
+    /**
+     * Simple copier that transfers all input to output.
+     */
+    private class TransferCopier implements Runnable {
 
         private final InputStream in;
         private final OutputStream out;
-        private final Integer logLevel;
+
+        public TransferCopier(InputStream in, OutputStream out) {
+            this.in = in;
+            this.out = out;
+        }
+
+        @Override
+        public void run() {
+            try {
+                byte[] data = new byte[1024];
+                int len;
+                while ((len = in.read(data)) >= 0) {
+                    out.write(data, 0, len);
+                    out.flush();
+                }
+            } catch (IOException  x) {
+                // ignore IOException: Broken pipe from FileOutputStream.writeBytes in BufferedOutputStream.flush
+            }
+        }
+
+    }
+
+    /**
+     * Filtering copier that marks up links, ignoring stack traces.
+     */
+    private class MarkupCopier implements Runnable {
+
+        private final InputStream in;
+        private final int logLevel;
         private final String encoding;
         private final RequestProcessor.Task flusher;
         private final ByteArrayOutputStream currentLine;
-        private OutputWriter ow = null;
-        private boolean err;
-        private AntSession session = null;
+        private final OutputWriter ow;
+        private final boolean err;
+        private final AntSession session;
         private final FoldingHelper foldingHelper;
 
-        public Copier(InputStream in, OutputStream out, Integer logLevel, String encoding/*, long init*/,
-                FoldingHelper foldingHelper) {
+        public MarkupCopier(InputStream in, int logLevel, boolean err, String encoding, NbBuildLogger buildLogger, FoldingHelper foldingHelper) {
             this.in = in;
-            this.out = out;
             this.logLevel = logLevel;
+            this.err = err;
             this.encoding = encoding;
             this.foldingHelper = foldingHelper;
-            if (logLevel != null) {
-                flusher = PROCESSOR.create(new Runnable() {
-                    public void run() {
-                        maybeFlush();
-                    }
-                });
-                currentLine = new ByteArrayOutputStream();
+
+            flusher = PROCESSOR.create(() -> maybeFlush(false));
+            currentLine = new ByteArrayOutputStream();
+
+            ow = err ? buildLogger.err : buildLogger.out;
+            session = buildLogger.thisSession;
+        }
+
+        private synchronized void append(byte[] data, int off, int len) {
+            currentLine.write(data, off, len);
+            if (currentLine.size() > 8192) {
+                flusher.run();
             } else {
-                flusher = null;
-                currentLine = null;
+                flusher.schedule(250);
             }
         }
 
+        private synchronized String appendAndTake(byte[] data, int off, int len) throws UnsupportedEncodingException {
+            currentLine.write(data, off, len);
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
+        private synchronized String take() throws UnsupportedEncodingException {
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
         public void run() {
-            /*
-            StringBuilder content = new StringBuilder();
-            long tick = System.currentTimeMillis();
-            content.append(String.format("[init: %1.1fsec]", (tick - init) / 1000.0));
-             */
-            
-            if (ow == null && logLevel != null) {
-                Vector v = getProject().getBuildListeners();
-                for (Object o : v) {
-                    if (o instanceof NbBuildLogger) {
-                        NbBuildLogger l = (NbBuildLogger) o;
-                        err = logLevel != Project.MSG_INFO;
-                        ow = err ? l.err : l.out;
-                        session = l.thisSession;
-                        break;
-                    }
-                }
-            }
             try {
+                byte[] data = new byte[1024];
+                int len;
+
                 try {
-                    int c;
-                    while ((c = in.read()) != -1) {
-                        if (logLevel == null) {
-                            // Input gets sent immediately.
-                            out.write(c);
-                            out.flush();
-                        } else {
-                            synchronized (this) {
-                                if (c == '\n') {                                    
-                                    String str = currentLine.toString(encoding);
-                                    int len = str.length();
-                                    if (len > 0 && str.charAt(len - 1) == '\r') {
-                                        str = str.substring(0, len - 1);
-                                    }
+                    while ((len = in.read(data)) >= 0) {
+                        int last = 0;
+                        for (int i = 0; i < len; i++) {
+                            int c = data[i] & 0xff;
 
+                            // Add folds for stack traces and mark up lines
+                            // not processed by JavaAntLogger stack trace detection
+                            if (c == '\n') {
+                                String str = appendAndTake(data, last, i > last && data[i - 1] == '\r' ? i - last - 1 : i - last);
+
+                                synchronized (foldingHelper) {
                                     foldingHelper.checkFolds(str, err, session);
-                                    if (str.length() < LOGGER_MAX_LINE_LENGTH) { // not too long message, probably interesting
-                                        // skip stack traces (hyperlinks are created by JavaAntLogger), everything else write directly
-                                        if (!STACK_TRACE.matcher(str).find()) {
-                                            StandardLogger.findHyperlink(str, session, null).println(session, err);
-                                        }
-                                    } else {
-                                        // do not match long strings, directly create a trivial hyperlink
+                                    if (str.length() >= LOGGER_MAX_LINE_LENGTH || !STACK_TRACE.matcher(str).matches())
                                         StandardLogger.findHyperlink(str, session, null).println(session, err);

Review Comment:
   gonna have to check if matches() gives the same results as find() with the given regex.



##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                        tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                        tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                } else {
+                    setCopier(stdout, getOutputStream(), delegateOutputStream, false);
+                    setCopier(stderr, getErrorStream(), delegateErrorStream, true);
+                }                                

Review Comment:
   small nitpick: indentation



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#issuecomment-1144230175

   @notzed your github name is fine. I believe Eric meant the name on the commit which is "Not Zed". This has to be a real person I believe, with a correct mail address.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] notzed commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
notzed commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r898606025


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                    tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                    tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                } else {
+                    setCopier(stdout, getOutputStream(), delegateOutputStream, false);
+                    setCopier(stderr, getErrorStream(), delegateErrorStream, true);
+                }                                
+                InputStream is = getInputStream();
+                if (is == null)
+                    is = AntBridge.delegateInputStream();
+                inputTask = tasks.submit(new TransferCopier(is, stdin));
+            }
 
             public void stop() {
-                if (errTask != null) {
-                    try {
-                        errTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outTask != null) {
-                    try {
-                        outTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (inTask != null) {
-                    inTask.interrupt();
-                    try {
-                        inTask.join(1000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outCopier != null) {
-                    outCopier.maybeFlush();
-                }
-                if (errCopier != null) {
-                    errCopier.maybeFlush();
+                try {
+                    if (inputTask != null)
+                        inputTask.cancel(true);
+                    tasks.shutdown();
+                    tasks.awaitTermination(3, TimeUnit.SECONDS);
+                } catch (InterruptedException ex) {
+                } finally {
+                    tasks.shutdownNow();
                 }
             }
 
             public void setProcessOutputStream(InputStream inputStream) throws IOException {
-                OutputStream os = getOutputStream();
-                Integer logLevel = null;
-                if (os == null || delegateOutputStream) {
-                    os = AntBridge.delegateOutputStream(false);
-                    logLevel = Project.MSG_INFO;
-                }
-                outTask = new Thread(Thread.currentThread().getThreadGroup(), outCopier = new Copier(inputStream, os, logLevel, outEncoding, foldingHelper),
-                        "Out Thread for " + getProject().getName()); // NOI18N
-                outTask.setDaemon(true);
-                outTask.start();
+                this.stdout = inputStream;
             }
 
             public void setProcessErrorStream(InputStream inputStream) throws IOException {
-                OutputStream os = getErrorStream();
-                Integer logLevel = null;
-                if (os == null || delegateErrorStream) {
-                    os = AntBridge.delegateOutputStream(true);
-                    logLevel = Project.MSG_WARN;
-                }
-                errTask = new Thread(Thread.currentThread().getThreadGroup(), errCopier = new Copier(inputStream, os, logLevel, errEncoding, foldingHelper),
-                        "Err Thread for " + getProject().getName()); // NOI18N
-                errTask.setDaemon(true);
-                errTask.start();
+                this.stderr = inputStream;
             }
 
             public void setProcessInputStream(OutputStream outputStream) throws IOException {
-                InputStream is = getInputStream();
-                if (is == null) {
-                    is = AntBridge.delegateInputStream();
-                }
-                inTask = new Thread(Thread.currentThread().getThreadGroup(), new Copier(is, outputStream, null, null, foldingHelper),
-                        "In Thread for " + getProject().getName()); // NOI18N
-                inTask.setDaemon(true);
-                inTask.start();
+                this.stdin = outputStream;
             }
 
         }
 
     }
 
-    private class Copier implements Runnable {
+    /**
+     * Simple copier that transfers all input to output.
+     */
+    private class TransferCopier implements Runnable {
 
         private final InputStream in;
         private final OutputStream out;
-        private final Integer logLevel;
+
+        public TransferCopier(InputStream in, OutputStream out) {
+            this.in = in;
+            this.out = out;
+        }
+
+        @Override
+        public void run() {
+            try {
+                byte[] data = new byte[1024];
+                int len;
+                while ((len = in.read(data)) >= 0) {
+                    out.write(data, 0, len);
+                    out.flush();
+                }
+            } catch (IOException  x) {
+                // ignore IOException: Broken pipe from FileOutputStream.writeBytes in BufferedOutputStream.flush
+            }
+        }
+
+    }
+
+    /**
+     * Filtering copier that marks up links, ignoring stack traces.
+     */
+    private class MarkupCopier implements Runnable {
+
+        private final InputStream in;
+        private final int logLevel;
         private final String encoding;
         private final RequestProcessor.Task flusher;
         private final ByteArrayOutputStream currentLine;
-        private OutputWriter ow = null;
-        private boolean err;
-        private AntSession session = null;
+        private final OutputWriter ow;
+        private final boolean err;
+        private final AntSession session;
         private final FoldingHelper foldingHelper;
 
-        public Copier(InputStream in, OutputStream out, Integer logLevel, String encoding/*, long init*/,
-                FoldingHelper foldingHelper) {
+        public MarkupCopier(InputStream in, int logLevel, boolean err, String encoding, NbBuildLogger buildLogger, FoldingHelper foldingHelper) {
             this.in = in;
-            this.out = out;
             this.logLevel = logLevel;
+            this.err = err;
             this.encoding = encoding;
             this.foldingHelper = foldingHelper;
-            if (logLevel != null) {
-                flusher = PROCESSOR.create(new Runnable() {
-                    public void run() {
-                        maybeFlush();
-                    }
-                });
-                currentLine = new ByteArrayOutputStream();
+
+            flusher = PROCESSOR.create(() -> maybeFlush(false));
+            currentLine = new ByteArrayOutputStream();
+
+            ow = err ? buildLogger.err : buildLogger.out;
+            session = buildLogger.thisSession;
+        }
+
+        private synchronized void append(byte[] data, int off, int len) {
+            currentLine.write(data, off, len);
+            if (currentLine.size() > 8192) {
+                flusher.run();
             } else {
-                flusher = null;
-                currentLine = null;
+                flusher.schedule(250);
             }
         }
 
+        private synchronized String appendAndTake(byte[] data, int off, int len) throws UnsupportedEncodingException {
+            currentLine.write(data, off, len);
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
+        private synchronized String take() throws UnsupportedEncodingException {
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
         public void run() {
-            /*
-            StringBuilder content = new StringBuilder();
-            long tick = System.currentTimeMillis();
-            content.append(String.format("[init: %1.1fsec]", (tick - init) / 1000.0));
-             */
-            
-            if (ow == null && logLevel != null) {
-                Vector v = getProject().getBuildListeners();
-                for (Object o : v) {
-                    if (o instanceof NbBuildLogger) {
-                        NbBuildLogger l = (NbBuildLogger) o;
-                        err = logLevel != Project.MSG_INFO;
-                        ow = err ? l.err : l.out;
-                        session = l.thisSession;
-                        break;
-                    }
-                }
-            }
             try {
+                byte[] data = new byte[1024];
+                int len;
+
                 try {
-                    int c;
-                    while ((c = in.read()) != -1) {
-                        if (logLevel == null) {
-                            // Input gets sent immediately.
-                            out.write(c);
-                            out.flush();
-                        } else {
-                            synchronized (this) {
-                                if (c == '\n') {                                    
-                                    String str = currentLine.toString(encoding);
-                                    int len = str.length();
-                                    if (len > 0 && str.charAt(len - 1) == '\r') {
-                                        str = str.substring(0, len - 1);
-                                    }
+                    while ((len = in.read(data)) >= 0) {
+                        int last = 0;
+                        for (int i = 0; i < len; i++) {
+                            int c = data[i] & 0xff;
 
+                            // Add folds for stack traces and mark up lines
+                            // not processed by JavaAntLogger stack trace detection
+                            if (c == '\n') {
+                                String str = appendAndTake(data, last, i > last && data[i - 1] == '\r' ? i - last - 1 : i - last);
+
+                                synchronized (foldingHelper) {
                                     foldingHelper.checkFolds(str, err, session);
-                                    if (str.length() < LOGGER_MAX_LINE_LENGTH) { // not too long message, probably interesting
-                                        // skip stack traces (hyperlinks are created by JavaAntLogger), everything else write directly
-                                        if (!STACK_TRACE.matcher(str).find()) {
-                                            StandardLogger.findHyperlink(str, session, null).println(session, err);
-                                        }
-                                    } else {
-                                        // do not match long strings, directly create a trivial hyperlink
+                                    if (str.length() >= LOGGER_MAX_LINE_LENGTH || !STACK_TRACE.matcher(str).matches())
                                         StandardLogger.findHyperlink(str, session, null).println(session, err);
-                                    }
                                     log(str, logLevel);
-                                    currentLine.reset();
-                                } else {                                    
-                                    currentLine.write(c);
-                                    if(currentLine.size() > 8192) {
-                                        flusher.run();
-                                    } else {
-                                        flusher.schedule(250);
-                                    }
                                 }
-                            }    
+                                last = i + 1;
+                            }
                         }
+
+                        if (last < len)
+                            append(data, last, len - last);
                     }
                 } finally {
-                    if (logLevel != null) {
-                        maybeFlush();
-                        if (err) {
-                            foldingHelper.clearHandle();
-                        }
-                    }
+                    maybeFlush(true);
                 }
             } catch (IOException x) {
                 // ignore IOException: Broken pipe from FileOutputStream.writeBytes in BufferedOutputStream.flush
-            } catch (ThreadDeath d) {
-                // OK, build just stopped.
-                return;
             }
-            //System.err.println("copied " + in + " to " + out + "; content='" + content + "'");
         }
 
-        private synchronized void maybeFlush() {
-            if (ow == null) { // ?? #200365
-                return;
-            }
+        public void maybeFlush(boolean end) {
             try {
-                if (currentLine.size() > 0) {
-                    String str = currentLine.toString(encoding);
-                    ow.write(str);
-                    log(str, logLevel);
+                String str = take();
+                synchronized (foldingHelper) {
+                    if (!str.isEmpty()) {
+                        ow.write(str);
+                        log(str, logLevel);
+                    }
+                    if (end && err)
+                        foldingHelper.clearHandle();
                 }

Review Comment:
   It's a bit ugly but while i was cleaning up the final patch I noticed the comment on FoldingHelper stated it was supposed to be used as a lock for interleaving output, but then it wasn't actually used.  
   
   With this changed it will ensure stderr/stdout has full line outputs before interleaving unless the timeout/flush happens in-between.  The output from the case below is improved although each line still interleaves due to the output rate.
   
       public class SlowOutput {
       static void deepStackTrace(int i) {
           if (i == 0) {
               try {
                   System.out.println("/home/notzed/.emacs:5:4: yo ho");
                   throw new Exception();
               } catch (Exception x) {
                   x.printStackTrace(System.err);
               }
           } else {
               deepStackTrace(i - 1);
           }
       }
       public static void main(String[] args) throws InterruptedException {
           for (int i = 0; i < 10000; i++) {
               for (int j = 0; j < 1000; j++) {
                   System.out.print(j);
               }
               System.out.println();
               if (i % 50 == 49)   deepStackTrace(5);
           }
       }
       }
   
   



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r898610350


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                    tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                    tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                } else {
+                    setCopier(stdout, getOutputStream(), delegateOutputStream, false);
+                    setCopier(stderr, getErrorStream(), delegateErrorStream, true);
+                }                                
+                InputStream is = getInputStream();
+                if (is == null)
+                    is = AntBridge.delegateInputStream();
+                inputTask = tasks.submit(new TransferCopier(is, stdin));
+            }
 
             public void stop() {
-                if (errTask != null) {
-                    try {
-                        errTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outTask != null) {
-                    try {
-                        outTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (inTask != null) {
-                    inTask.interrupt();
-                    try {
-                        inTask.join(1000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outCopier != null) {
-                    outCopier.maybeFlush();
-                }
-                if (errCopier != null) {
-                    errCopier.maybeFlush();
+                try {
+                    if (inputTask != null)
+                        inputTask.cancel(true);
+                    tasks.shutdown();
+                    tasks.awaitTermination(3, TimeUnit.SECONDS);
+                } catch (InterruptedException ex) {
+                } finally {
+                    tasks.shutdownNow();
                 }
             }
 
             public void setProcessOutputStream(InputStream inputStream) throws IOException {
-                OutputStream os = getOutputStream();
-                Integer logLevel = null;
-                if (os == null || delegateOutputStream) {
-                    os = AntBridge.delegateOutputStream(false);
-                    logLevel = Project.MSG_INFO;
-                }
-                outTask = new Thread(Thread.currentThread().getThreadGroup(), outCopier = new Copier(inputStream, os, logLevel, outEncoding, foldingHelper),
-                        "Out Thread for " + getProject().getName()); // NOI18N
-                outTask.setDaemon(true);
-                outTask.start();
+                this.stdout = inputStream;
             }
 
             public void setProcessErrorStream(InputStream inputStream) throws IOException {
-                OutputStream os = getErrorStream();
-                Integer logLevel = null;
-                if (os == null || delegateErrorStream) {
-                    os = AntBridge.delegateOutputStream(true);
-                    logLevel = Project.MSG_WARN;
-                }
-                errTask = new Thread(Thread.currentThread().getThreadGroup(), errCopier = new Copier(inputStream, os, logLevel, errEncoding, foldingHelper),
-                        "Err Thread for " + getProject().getName()); // NOI18N
-                errTask.setDaemon(true);
-                errTask.start();
+                this.stderr = inputStream;
             }
 
             public void setProcessInputStream(OutputStream outputStream) throws IOException {
-                InputStream is = getInputStream();
-                if (is == null) {
-                    is = AntBridge.delegateInputStream();
-                }
-                inTask = new Thread(Thread.currentThread().getThreadGroup(), new Copier(is, outputStream, null, null, foldingHelper),
-                        "In Thread for " + getProject().getName()); // NOI18N
-                inTask.setDaemon(true);
-                inTask.start();
+                this.stdin = outputStream;
             }
 
         }
 
     }
 
-    private class Copier implements Runnable {
+    /**
+     * Simple copier that transfers all input to output.
+     */
+    private class TransferCopier implements Runnable {
 
         private final InputStream in;
         private final OutputStream out;
-        private final Integer logLevel;
+
+        public TransferCopier(InputStream in, OutputStream out) {
+            this.in = in;
+            this.out = out;
+        }
+
+        @Override
+        public void run() {
+            try {
+                byte[] data = new byte[1024];
+                int len;
+                while ((len = in.read(data)) >= 0) {
+                    out.write(data, 0, len);
+                    out.flush();
+                }
+            } catch (IOException  x) {
+                // ignore IOException: Broken pipe from FileOutputStream.writeBytes in BufferedOutputStream.flush
+            }
+        }
+
+    }
+
+    /**
+     * Filtering copier that marks up links, ignoring stack traces.
+     */
+    private class MarkupCopier implements Runnable {
+
+        private final InputStream in;
+        private final int logLevel;
         private final String encoding;
         private final RequestProcessor.Task flusher;
         private final ByteArrayOutputStream currentLine;
-        private OutputWriter ow = null;
-        private boolean err;
-        private AntSession session = null;
+        private final OutputWriter ow;
+        private final boolean err;
+        private final AntSession session;
         private final FoldingHelper foldingHelper;
 
-        public Copier(InputStream in, OutputStream out, Integer logLevel, String encoding/*, long init*/,
-                FoldingHelper foldingHelper) {
+        public MarkupCopier(InputStream in, int logLevel, boolean err, String encoding, NbBuildLogger buildLogger, FoldingHelper foldingHelper) {
             this.in = in;
-            this.out = out;
             this.logLevel = logLevel;
+            this.err = err;
             this.encoding = encoding;
             this.foldingHelper = foldingHelper;
-            if (logLevel != null) {
-                flusher = PROCESSOR.create(new Runnable() {
-                    public void run() {
-                        maybeFlush();
-                    }
-                });
-                currentLine = new ByteArrayOutputStream();
+
+            flusher = PROCESSOR.create(() -> maybeFlush(false));
+            currentLine = new ByteArrayOutputStream();
+
+            ow = err ? buildLogger.err : buildLogger.out;
+            session = buildLogger.thisSession;
+        }
+
+        private synchronized void append(byte[] data, int off, int len) {
+            currentLine.write(data, off, len);
+            if (currentLine.size() > 8192) {
+                flusher.run();
             } else {
-                flusher = null;
-                currentLine = null;
+                flusher.schedule(250);
             }
         }
 
+        private synchronized String appendAndTake(byte[] data, int off, int len) throws UnsupportedEncodingException {
+            currentLine.write(data, off, len);
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
+        private synchronized String take() throws UnsupportedEncodingException {
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
         public void run() {
-            /*
-            StringBuilder content = new StringBuilder();
-            long tick = System.currentTimeMillis();
-            content.append(String.format("[init: %1.1fsec]", (tick - init) / 1000.0));
-             */
-            
-            if (ow == null && logLevel != null) {
-                Vector v = getProject().getBuildListeners();
-                for (Object o : v) {
-                    if (o instanceof NbBuildLogger) {
-                        NbBuildLogger l = (NbBuildLogger) o;
-                        err = logLevel != Project.MSG_INFO;
-                        ow = err ? l.err : l.out;
-                        session = l.thisSession;
-                        break;
-                    }
-                }
-            }
             try {
+                byte[] data = new byte[1024];
+                int len;
+
                 try {
-                    int c;
-                    while ((c = in.read()) != -1) {
-                        if (logLevel == null) {
-                            // Input gets sent immediately.
-                            out.write(c);
-                            out.flush();
-                        } else {
-                            synchronized (this) {
-                                if (c == '\n') {                                    
-                                    String str = currentLine.toString(encoding);
-                                    int len = str.length();
-                                    if (len > 0 && str.charAt(len - 1) == '\r') {
-                                        str = str.substring(0, len - 1);
-                                    }
+                    while ((len = in.read(data)) >= 0) {
+                        int last = 0;
+                        for (int i = 0; i < len; i++) {
+                            int c = data[i] & 0xff;
 
+                            // Add folds for stack traces and mark up lines
+                            // not processed by JavaAntLogger stack trace detection
+                            if (c == '\n') {
+                                String str = appendAndTake(data, last, i > last && data[i - 1] == '\r' ? i - last - 1 : i - last);
+
+                                synchronized (foldingHelper) {
                                     foldingHelper.checkFolds(str, err, session);
-                                    if (str.length() < LOGGER_MAX_LINE_LENGTH) { // not too long message, probably interesting
-                                        // skip stack traces (hyperlinks are created by JavaAntLogger), everything else write directly
-                                        if (!STACK_TRACE.matcher(str).find()) {
-                                            StandardLogger.findHyperlink(str, session, null).println(session, err);
-                                        }
-                                    } else {
-                                        // do not match long strings, directly create a trivial hyperlink
+                                    if (str.length() >= LOGGER_MAX_LINE_LENGTH || !STACK_TRACE.matcher(str).matches())
                                         StandardLogger.findHyperlink(str, session, null).println(session, err);
-                                    }
                                     log(str, logLevel);
-                                    currentLine.reset();
-                                } else {                                    
-                                    currentLine.write(c);
-                                    if(currentLine.size() > 8192) {
-                                        flusher.run();
-                                    } else {
-                                        flusher.schedule(250);
-                                    }
                                 }
-                            }    
+                                last = i + 1;
+                            }
                         }
+
+                        if (last < len)
+                            append(data, last, len - last);
                     }
                 } finally {
-                    if (logLevel != null) {
-                        maybeFlush();
-                        if (err) {
-                            foldingHelper.clearHandle();
-                        }
-                    }
+                    maybeFlush(true);
                 }
             } catch (IOException x) {
                 // ignore IOException: Broken pipe from FileOutputStream.writeBytes in BufferedOutputStream.flush
-            } catch (ThreadDeath d) {
-                // OK, build just stopped.
-                return;
             }
-            //System.err.println("copied " + in + " to " + out + "; content='" + content + "'");
         }
 
-        private synchronized void maybeFlush() {
-            if (ow == null) { // ?? #200365
-                return;
-            }
+        public void maybeFlush(boolean end) {
             try {
-                if (currentLine.size() > 0) {
-                    String str = currentLine.toString(encoding);
-                    ow.write(str);
-                    log(str, logLevel);
+                String str = take();
+                synchronized (foldingHelper) {
+                    if (!str.isEmpty()) {
+                        ow.write(str);
+                        log(str, logLevel);
+                    }
+                    if (end && err)
+                        foldingHelper.clearHandle();
                 }

Review Comment:
   good stuff!



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#issuecomment-1158129663

   planning to merge this soon unless someone speaks up @lkishalmi @matthiasblaesing 


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] notzed commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
notzed commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r889634527


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                        tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                        tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                } else {
+                    setCopier(stdout, getOutputStream(), delegateOutputStream, false);
+                    setCopier(stderr, getErrorStream(), delegateErrorStream, true);
+                }                                

Review Comment:
   Ok I'll fix this up, i've been checking diffs to avoid whitespace changes but i need new glasses (and to wear them).
   
   I've been fighting with netbeans on this, every time i start typing the indenting is super-wrong so out of habit I use autoformat constantly, and i can't find the settings that match the existing code.  So if i run it I have to undo a bunch of things it changes (apart from the settings being global and messing with all my other projects), it's pretty tedious since in addition i'm a tabs guy and spaces take so long to traverse.
   



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r889637652


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                        tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                        tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                } else {
+                    setCopier(stdout, getOutputStream(), delegateOutputStream, false);
+                    setCopier(stderr, getErrorStream(), delegateErrorStream, true);
+                }                                

Review Comment:
   you can actually configure it on a per-project basis which would override the global default. But NetBeans has ~840 sub projects so its probably not worth the trouble :)
   
   I think the only change I made from the global default is to set tab size to 4 (I believe 8 is default). But this would only show in files which actually use tabs.
   
   I rarely use auto formatting. If i do its usually only for few selected lines since it creates too much noise in the diff.
   
   It was not super important anyway, I only mentioned it since I noticed it while looking through it. Thanks for fixing it.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] notzed commented on pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
notzed commented on PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#issuecomment-1159588976

   Goodo, been a while since I contributed much to anything but i was very active in GNOME back in the day (2nd iteration of gnome-terminal, evolution).  Dunno if i'll find something else to poke around in netbeans but we'll see - i quit my job 18 months ago and can't be bothered getting another one yet so i've got a lot of time on my hands and it's getting a bit boring.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r886792318


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,228 +166,222 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst().ifPresentOrElse(buildLogger -> {
+                        tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                        tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                    }, ()->{

Review Comment:
   `ifPresentOrElse` is Java 9. Some modules have to stay compatible with Java 8 still, unfortunately. (thats the reason why the build fails)



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] notzed commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
notzed commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r889776437


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                        tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                        tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                } else {
+                    setCopier(stdout, getOutputStream(), delegateOutputStream, false);
+                    setCopier(stderr, getErrorStream(), delegateErrorStream, true);
+                }                                
+                InputStream is = getInputStream();
+                if (is == null)
+                    is = AntBridge.delegateInputStream();
+                inputTask = tasks.submit(new TransferCopier(is, stdin));
+            }
 
             public void stop() {
-                if (errTask != null) {
-                    try {
-                        errTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outTask != null) {
-                    try {
-                        outTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (inTask != null) {
-                    inTask.interrupt();
-                    try {
-                        inTask.join(1000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outCopier != null) {
-                    outCopier.maybeFlush();
-                }
-                if (errCopier != null) {
-                    errCopier.maybeFlush();
+                try {
+                    if (inputTask != null)
+                        inputTask.cancel(true);
+                    tasks.shutdown();
+                    tasks.awaitTermination(3, TimeUnit.SECONDS);
+                } catch (InterruptedException ex) {
+                } finally {
+                    tasks.shutdownNow();
                 }
             }
 
             public void setProcessOutputStream(InputStream inputStream) throws IOException {
-                OutputStream os = getOutputStream();
-                Integer logLevel = null;
-                if (os == null || delegateOutputStream) {
-                    os = AntBridge.delegateOutputStream(false);
-                    logLevel = Project.MSG_INFO;
-                }
-                outTask = new Thread(Thread.currentThread().getThreadGroup(), outCopier = new Copier(inputStream, os, logLevel, outEncoding, foldingHelper),
-                        "Out Thread for " + getProject().getName()); // NOI18N
-                outTask.setDaemon(true);
-                outTask.start();
+                this.stdout = inputStream;
             }
 
             public void setProcessErrorStream(InputStream inputStream) throws IOException {
-                OutputStream os = getErrorStream();
-                Integer logLevel = null;
-                if (os == null || delegateErrorStream) {
-                    os = AntBridge.delegateOutputStream(true);
-                    logLevel = Project.MSG_WARN;
-                }
-                errTask = new Thread(Thread.currentThread().getThreadGroup(), errCopier = new Copier(inputStream, os, logLevel, errEncoding, foldingHelper),
-                        "Err Thread for " + getProject().getName()); // NOI18N
-                errTask.setDaemon(true);
-                errTask.start();
+                this.stderr = inputStream;
             }
 
             public void setProcessInputStream(OutputStream outputStream) throws IOException {
-                InputStream is = getInputStream();
-                if (is == null) {
-                    is = AntBridge.delegateInputStream();
-                }
-                inTask = new Thread(Thread.currentThread().getThreadGroup(), new Copier(is, outputStream, null, null, foldingHelper),
-                        "In Thread for " + getProject().getName()); // NOI18N
-                inTask.setDaemon(true);
-                inTask.start();
+                this.stdin = outputStream;
             }
 
         }
 
     }
 
-    private class Copier implements Runnable {
+    /**
+     * Simple copier that transfers all input to output.
+     */
+    private class TransferCopier implements Runnable {
 
         private final InputStream in;
         private final OutputStream out;
-        private final Integer logLevel;
+
+        public TransferCopier(InputStream in, OutputStream out) {
+            this.in = in;
+            this.out = out;
+        }
+
+        @Override
+        public void run() {
+            try {
+                byte[] data = new byte[1024];
+                int len;
+                while ((len = in.read(data)) >= 0) {
+                    out.write(data, 0, len);
+                    out.flush();
+                }
+            } catch (IOException  x) {
+                // ignore IOException: Broken pipe from FileOutputStream.writeBytes in BufferedOutputStream.flush
+            }
+        }
+
+    }
+
+    /**
+     * Filtering copier that marks up links, ignoring stack traces.
+     */
+    private class MarkupCopier implements Runnable {
+
+        private final InputStream in;
+        private final int logLevel;
         private final String encoding;
         private final RequestProcessor.Task flusher;
         private final ByteArrayOutputStream currentLine;
-        private OutputWriter ow = null;
-        private boolean err;
-        private AntSession session = null;
+        private final OutputWriter ow;
+        private final boolean err;
+        private final AntSession session;
         private final FoldingHelper foldingHelper;
 
-        public Copier(InputStream in, OutputStream out, Integer logLevel, String encoding/*, long init*/,
-                FoldingHelper foldingHelper) {
+        public MarkupCopier(InputStream in, int logLevel, boolean err, String encoding, NbBuildLogger buildLogger, FoldingHelper foldingHelper) {
             this.in = in;
-            this.out = out;
             this.logLevel = logLevel;
+            this.err = err;
             this.encoding = encoding;
             this.foldingHelper = foldingHelper;
-            if (logLevel != null) {
-                flusher = PROCESSOR.create(new Runnable() {
-                    public void run() {
-                        maybeFlush();
-                    }
-                });
-                currentLine = new ByteArrayOutputStream();
+
+            flusher = PROCESSOR.create(() -> maybeFlush(false));
+            currentLine = new ByteArrayOutputStream();
+
+            ow = err ? buildLogger.err : buildLogger.out;
+            session = buildLogger.thisSession;
+        }
+
+        private synchronized void append(byte[] data, int off, int len) {
+            currentLine.write(data, off, len);
+            if (currentLine.size() > 8192) {
+                flusher.run();
             } else {
-                flusher = null;
-                currentLine = null;
+                flusher.schedule(250);
             }
         }
 
+        private synchronized String appendAndTake(byte[] data, int off, int len) throws UnsupportedEncodingException {
+            currentLine.write(data, off, len);
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
+        private synchronized String take() throws UnsupportedEncodingException {
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
         public void run() {
-            /*
-            StringBuilder content = new StringBuilder();
-            long tick = System.currentTimeMillis();
-            content.append(String.format("[init: %1.1fsec]", (tick - init) / 1000.0));
-             */
-            
-            if (ow == null && logLevel != null) {
-                Vector v = getProject().getBuildListeners();
-                for (Object o : v) {
-                    if (o instanceof NbBuildLogger) {
-                        NbBuildLogger l = (NbBuildLogger) o;
-                        err = logLevel != Project.MSG_INFO;
-                        ow = err ? l.err : l.out;
-                        session = l.thisSession;
-                        break;
-                    }
-                }
-            }
             try {
+                byte[] data = new byte[1024];
+                int len;
+
                 try {
-                    int c;
-                    while ((c = in.read()) != -1) {
-                        if (logLevel == null) {
-                            // Input gets sent immediately.
-                            out.write(c);
-                            out.flush();
-                        } else {
-                            synchronized (this) {
-                                if (c == '\n') {                                    
-                                    String str = currentLine.toString(encoding);
-                                    int len = str.length();
-                                    if (len > 0 && str.charAt(len - 1) == '\r') {
-                                        str = str.substring(0, len - 1);
-                                    }
+                    while ((len = in.read(data)) >= 0) {
+                        int last = 0;
+                        for (int i = 0; i < len; i++) {
+                            int c = data[i] & 0xff;
 
+                            // Add folds for stack traces and mark up lines
+                            // not processed by JavaAntLogger stack trace detection
+                            if (c == '\n') {
+                                String str = appendAndTake(data, last, i > last && data[i - 1] == '\r' ? i - last - 1 : i - last);
+
+                                synchronized (foldingHelper) {
                                     foldingHelper.checkFolds(str, err, session);
-                                    if (str.length() < LOGGER_MAX_LINE_LENGTH) { // not too long message, probably interesting
-                                        // skip stack traces (hyperlinks are created by JavaAntLogger), everything else write directly
-                                        if (!STACK_TRACE.matcher(str).find()) {
-                                            StandardLogger.findHyperlink(str, session, null).println(session, err);
-                                        }
-                                    } else {
-                                        // do not match long strings, directly create a trivial hyperlink
+                                    if (str.length() >= LOGGER_MAX_LINE_LENGTH || !STACK_TRACE.matcher(str).matches())
                                         StandardLogger.findHyperlink(str, session, null).println(session, err);

Review Comment:
   For the test i was using, find vs matches is roughly ~1500x slower and there are worse cases.  Due to the greedy operator it matches the whole string then walks all the way back to the beginning testing each position looking for the next substring match.  With matches() it then terminates but because it isn't anchored to the start of the string find() advances by one character and repeats - all the way to the other end - so it's particularly bad with long lines, and is the reason for the line length checking here and in javaantlogger. I filed a jdk performance issue against java.util.regex.Pattern but my initial assessment was wrong as to the cause, and it would be tricky to address due to the way the state is managed in the compiled expression.
   
   perl doesn't even execute the regex because it does a pre-scan and determines the string can't possibly match due to the bits that aren't optional which are not present like '('.
   
   Removing the .* at either end makes find() run about the same as matches().  With some simplification and no capturing groups it can run somewhat faster than calling matches() but it's no longer a significant part of the processing time and doesn't seem worth maintaining another equally unreadable expression.
   



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#issuecomment-1159253564

   oh no @notzed ;) don't merge into this branch. Now we have a 
   `Author: notzed <51...@users.noreply.github.com>` commit in here.
   
   You think you can revert that merge and only keep the commit?
   `git reset HEAD^`
   should remove the last commit. And simply force push again.
   
   In future if you want to refresh PRs you can do something like this:
   `git pull --rebase --autostash origin master`
   this is going to put the commit on top of latest master. (assuming origin points at the main repo, but in general its only needed to refresh PRs when they are a few months old or have conflicts)


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] notzed commented on pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
notzed commented on PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#issuecomment-1159349127

   I must've read somewhere to keep the fork up to date so i've been doing that, i wondered why the last time did something funny - the git told me to run some commands so i did and that's what must've done it.
   
   Err I think i fubard something else now.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r898609891


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                    tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                    tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));

Review Comment:
   feel free to apply the diff or implement it yourself, whatever you prefer. I simply refactored it to check if it would work and pasted my version.
   
   You can add it to the commit and force push into the PR branch.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r897480701


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                    tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                    tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));

Review Comment:
   second task should use `errEncoding`.
   
   i think we could also store the encoding as Charset, this would remove lookups inside toString(... "encoding"). 
   
   ```diff
   diff --git a/extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java b/extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java
   index b9fbad1..4f91cca 100644
   --- a/extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java
   +++ b/extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java
   @@ -25,6 +25,7 @@
    import java.io.InputStream;
    import java.io.OutputStream;
    import java.io.UnsupportedEncodingException;
   +import java.nio.charset.Charset;
    import java.util.concurrent.ExecutorService;
    import java.util.concurrent.Executors;
    import java.util.concurrent.Future;
   @@ -104,8 +105,8 @@
    
        private class NbRedirector extends Redirector {
    
   -        private String outEncoding = System.getProperty("file.encoding"); // NOI18N
   -        private String errEncoding = System.getProperty("file.encoding"); // NOI18N
   +        private Charset outEncoding;
   +        private Charset errEncoding;
    
            // #158492. In Ant 1.8.0 output redirection cannot be distinguished by OutputStream subclass type
            // (LogOutputStream vs OutputStreamFunneler$Funnel) as OutputStreamFunneler$Funnel is
   @@ -115,6 +116,14 @@
    
            public NbRedirector(Task task) {
                super(task);
   +            try {
   +                Charset cs = Charset.forName(System.getProperty("file.encoding")); // NOI18N
   +                outEncoding = cs;
   +                errEncoding = cs;
   +            } catch (IllegalArgumentException ex) {
   +                outEncoding = Charset.defaultCharset();
   +                errEncoding = Charset.defaultCharset();
   +            }
            }
    
            public @Override ExecuteStreamHandler createHandler() throws BuildException {
   @@ -123,12 +132,12 @@
            }
    
            public @Override synchronized void setOutputEncoding(String outputEncoding) {
   -            outEncoding = outputEncoding;
   +            outEncoding = Charset.forName(outputEncoding);
                super.setOutputEncoding(outputEncoding);
            }
    
            public @Override synchronized void setErrorEncoding(String errorEncoding) {
   -            errEncoding = errorEncoding;
   +            errEncoding = Charset.forName(errorEncoding);
                super.setErrorEncoding(errorEncoding);
            }
    
   @@ -199,7 +208,7 @@
                        .orElse(null);
                    if (buildLogger != null) {
                        tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
   -                    tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
   +                    tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, errEncoding, buildLogger, foldingHelper));
                    } else {
                        setCopier(stdout, getOutputStream(), delegateOutputStream, false);
                        setCopier(stderr, getErrorStream(), delegateErrorStream, true);
   @@ -274,7 +283,7 @@
    
            private final InputStream in;
            private final int logLevel;
   -        private final String encoding;
   +        private final Charset encoding;
            private final RequestProcessor.Task flusher;
            private final ByteArrayOutputStream currentLine;
            private final OutputWriter ow;
   @@ -282,7 +291,7 @@
            private final AntSession session;
            private final FoldingHelper foldingHelper;
    
   -        public MarkupCopier(InputStream in, int logLevel, boolean err, String encoding, NbBuildLogger buildLogger, FoldingHelper foldingHelper) {
   +        public MarkupCopier(InputStream in, int logLevel, boolean err, Charset encoding, NbBuildLogger buildLogger, FoldingHelper foldingHelper) {
                this.in = in;
                this.logLevel = logLevel;
                this.err = err;
   
   ```



##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                    tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                    tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                } else {
+                    setCopier(stdout, getOutputStream(), delegateOutputStream, false);
+                    setCopier(stderr, getErrorStream(), delegateErrorStream, true);
+                }                                
+                InputStream is = getInputStream();
+                if (is == null)
+                    is = AntBridge.delegateInputStream();
+                inputTask = tasks.submit(new TransferCopier(is, stdin));
+            }
 
             public void stop() {
-                if (errTask != null) {
-                    try {
-                        errTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outTask != null) {
-                    try {
-                        outTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (inTask != null) {
-                    inTask.interrupt();
-                    try {
-                        inTask.join(1000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outCopier != null) {
-                    outCopier.maybeFlush();
-                }
-                if (errCopier != null) {
-                    errCopier.maybeFlush();
+                try {
+                    if (inputTask != null)
+                        inputTask.cancel(true);
+                    tasks.shutdown();
+                    tasks.awaitTermination(3, TimeUnit.SECONDS);
+                } catch (InterruptedException ex) {
+                } finally {
+                    tasks.shutdownNow();
                 }
             }
 
             public void setProcessOutputStream(InputStream inputStream) throws IOException {
-                OutputStream os = getOutputStream();
-                Integer logLevel = null;
-                if (os == null || delegateOutputStream) {
-                    os = AntBridge.delegateOutputStream(false);
-                    logLevel = Project.MSG_INFO;
-                }
-                outTask = new Thread(Thread.currentThread().getThreadGroup(), outCopier = new Copier(inputStream, os, logLevel, outEncoding, foldingHelper),
-                        "Out Thread for " + getProject().getName()); // NOI18N
-                outTask.setDaemon(true);
-                outTask.start();
+                this.stdout = inputStream;
             }
 
             public void setProcessErrorStream(InputStream inputStream) throws IOException {
-                OutputStream os = getErrorStream();
-                Integer logLevel = null;
-                if (os == null || delegateErrorStream) {
-                    os = AntBridge.delegateOutputStream(true);
-                    logLevel = Project.MSG_WARN;
-                }
-                errTask = new Thread(Thread.currentThread().getThreadGroup(), errCopier = new Copier(inputStream, os, logLevel, errEncoding, foldingHelper),
-                        "Err Thread for " + getProject().getName()); // NOI18N
-                errTask.setDaemon(true);
-                errTask.start();
+                this.stderr = inputStream;
             }
 
             public void setProcessInputStream(OutputStream outputStream) throws IOException {
-                InputStream is = getInputStream();
-                if (is == null) {
-                    is = AntBridge.delegateInputStream();
-                }
-                inTask = new Thread(Thread.currentThread().getThreadGroup(), new Copier(is, outputStream, null, null, foldingHelper),
-                        "In Thread for " + getProject().getName()); // NOI18N
-                inTask.setDaemon(true);
-                inTask.start();
+                this.stdin = outputStream;
             }
 
         }
 
     }
 
-    private class Copier implements Runnable {
+    /**
+     * Simple copier that transfers all input to output.
+     */
+    private class TransferCopier implements Runnable {
 
         private final InputStream in;
         private final OutputStream out;
-        private final Integer logLevel;
+
+        public TransferCopier(InputStream in, OutputStream out) {
+            this.in = in;
+            this.out = out;
+        }
+
+        @Override
+        public void run() {
+            try {
+                byte[] data = new byte[1024];
+                int len;
+                while ((len = in.read(data)) >= 0) {
+                    out.write(data, 0, len);
+                    out.flush();
+                }
+            } catch (IOException  x) {
+                // ignore IOException: Broken pipe from FileOutputStream.writeBytes in BufferedOutputStream.flush
+            }
+        }
+
+    }
+
+    /**
+     * Filtering copier that marks up links, ignoring stack traces.
+     */
+    private class MarkupCopier implements Runnable {
+
+        private final InputStream in;
+        private final int logLevel;
         private final String encoding;
         private final RequestProcessor.Task flusher;
         private final ByteArrayOutputStream currentLine;
-        private OutputWriter ow = null;
-        private boolean err;
-        private AntSession session = null;
+        private final OutputWriter ow;
+        private final boolean err;
+        private final AntSession session;
         private final FoldingHelper foldingHelper;
 
-        public Copier(InputStream in, OutputStream out, Integer logLevel, String encoding/*, long init*/,
-                FoldingHelper foldingHelper) {
+        public MarkupCopier(InputStream in, int logLevel, boolean err, String encoding, NbBuildLogger buildLogger, FoldingHelper foldingHelper) {
             this.in = in;
-            this.out = out;
             this.logLevel = logLevel;
+            this.err = err;
             this.encoding = encoding;
             this.foldingHelper = foldingHelper;
-            if (logLevel != null) {
-                flusher = PROCESSOR.create(new Runnable() {
-                    public void run() {
-                        maybeFlush();
-                    }
-                });
-                currentLine = new ByteArrayOutputStream();
+
+            flusher = PROCESSOR.create(() -> maybeFlush(false));
+            currentLine = new ByteArrayOutputStream();
+
+            ow = err ? buildLogger.err : buildLogger.out;
+            session = buildLogger.thisSession;
+        }
+
+        private synchronized void append(byte[] data, int off, int len) {
+            currentLine.write(data, off, len);
+            if (currentLine.size() > 8192) {
+                flusher.run();
             } else {
-                flusher = null;
-                currentLine = null;
+                flusher.schedule(250);
             }
         }
 
+        private synchronized String appendAndTake(byte[] data, int off, int len) throws UnsupportedEncodingException {
+            currentLine.write(data, off, len);
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
+        private synchronized String take() throws UnsupportedEncodingException {
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
         public void run() {
-            /*
-            StringBuilder content = new StringBuilder();
-            long tick = System.currentTimeMillis();
-            content.append(String.format("[init: %1.1fsec]", (tick - init) / 1000.0));
-             */
-            
-            if (ow == null && logLevel != null) {
-                Vector v = getProject().getBuildListeners();
-                for (Object o : v) {
-                    if (o instanceof NbBuildLogger) {
-                        NbBuildLogger l = (NbBuildLogger) o;
-                        err = logLevel != Project.MSG_INFO;
-                        ow = err ? l.err : l.out;
-                        session = l.thisSession;
-                        break;
-                    }
-                }
-            }
             try {
+                byte[] data = new byte[1024];
+                int len;
+
                 try {
-                    int c;
-                    while ((c = in.read()) != -1) {
-                        if (logLevel == null) {
-                            // Input gets sent immediately.
-                            out.write(c);
-                            out.flush();
-                        } else {
-                            synchronized (this) {
-                                if (c == '\n') {                                    
-                                    String str = currentLine.toString(encoding);
-                                    int len = str.length();
-                                    if (len > 0 && str.charAt(len - 1) == '\r') {
-                                        str = str.substring(0, len - 1);
-                                    }
+                    while ((len = in.read(data)) >= 0) {
+                        int last = 0;
+                        for (int i = 0; i < len; i++) {
+                            int c = data[i] & 0xff;
 
+                            // Add folds for stack traces and mark up lines
+                            // not processed by JavaAntLogger stack trace detection
+                            if (c == '\n') {
+                                String str = appendAndTake(data, last, i > last && data[i - 1] == '\r' ? i - last - 1 : i - last);
+
+                                synchronized (foldingHelper) {
                                     foldingHelper.checkFolds(str, err, session);
-                                    if (str.length() < LOGGER_MAX_LINE_LENGTH) { // not too long message, probably interesting
-                                        // skip stack traces (hyperlinks are created by JavaAntLogger), everything else write directly
-                                        if (!STACK_TRACE.matcher(str).find()) {
-                                            StandardLogger.findHyperlink(str, session, null).println(session, err);
-                                        }
-                                    } else {
-                                        // do not match long strings, directly create a trivial hyperlink
+                                    if (str.length() >= LOGGER_MAX_LINE_LENGTH || !STACK_TRACE.matcher(str).matches())
                                         StandardLogger.findHyperlink(str, session, null).println(session, err);
-                                    }
                                     log(str, logLevel);
-                                    currentLine.reset();
-                                } else {                                    
-                                    currentLine.write(c);
-                                    if(currentLine.size() > 8192) {
-                                        flusher.run();
-                                    } else {
-                                        flusher.schedule(250);
-                                    }
                                 }
-                            }    
+                                last = i + 1;
+                            }
                         }
+
+                        if (last < len)
+                            append(data, last, len - last);
                     }
                 } finally {
-                    if (logLevel != null) {
-                        maybeFlush();
-                        if (err) {
-                            foldingHelper.clearHandle();
-                        }
-                    }
+                    maybeFlush(true);
                 }
             } catch (IOException x) {
                 // ignore IOException: Broken pipe from FileOutputStream.writeBytes in BufferedOutputStream.flush
-            } catch (ThreadDeath d) {
-                // OK, build just stopped.
-                return;
             }
-            //System.err.println("copied " + in + " to " + out + "; content='" + content + "'");
         }
 
-        private synchronized void maybeFlush() {
-            if (ow == null) { // ?? #200365
-                return;
-            }
+        public void maybeFlush(boolean end) {
             try {
-                if (currentLine.size() > 0) {
-                    String str = currentLine.toString(encoding);
-                    ow.write(str);
-                    log(str, logLevel);
+                String str = take();
+                synchronized (foldingHelper) {
+                    if (!str.isEmpty()) {
+                        ow.write(str);
+                        log(str, logLevel);
+                    }
+                    if (end && err)
+                        foldingHelper.clearHandle();
                 }

Review Comment:
   i like that you use `foldingHelper` as monitor instead of the task. This might fix a race condition, although interleaving `out` and `err` wouldn't result in proper folding anyway most of the time I suppose.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r898611578


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                    tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                    tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));

Review Comment:
   btw the way i see this: the exception would happen anyway, just later in the toString(... "encoding") method. So, converting it early to `Charset` should be fine without breaking contracts.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien merged pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien merged PR #4180:
URL: https://github.com/apache/netbeans/pull/4180


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] notzed commented on a diff in pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
notzed commented on code in PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#discussion_r889634003


##########
extide/o.apache.tools.ant.module/src-bridge/org/apache/tools/ant/module/bridge/impl/ForkedJavaOverride.java:
##########
@@ -162,219 +166,216 @@ public synchronized void setError(File[] error) {
 
         private class NbOutputStreamHandler implements ExecuteStreamHandler {
 
-            private Thread outTask;
-            private Thread errTask;
-            private Thread inTask;
-            private Copier outCopier, errCopier; // #212526
-            private FoldingHelper foldingHelper;
-
+            private final ExecutorService tasks;
+            private final FoldingHelper foldingHelper;
+            private Future inputTask;
+            private InputStream stdout, stderr;
+            private OutputStream stdin;
+            
             NbOutputStreamHandler() {
                 this.foldingHelper = new FoldingHelper();
+                this.tasks = Executors.newFixedThreadPool(3, (r) -> {
+                    Thread t = new Thread(Thread.currentThread().getThreadGroup(),
+                        r,
+                        "I/O Thread for " + getProject().getName()); // NOI18N
+                    t.setDaemon(true);
+                    return t;
+                });
             }
 
-            public void start() throws IOException {}
+            private void setCopier(InputStream inputStream, OutputStream os, boolean delegate, boolean err) {
+                if (os == null || delegate) {
+                    tasks.submit(new TransferCopier(inputStream, AntBridge.delegateOutputStream(err)));
+                } else {
+                    tasks.submit(new TransferCopier(inputStream, os));
+                }
+            }
+
+            public void start() throws IOException {
+                NbBuildLogger buildLogger = getProject().getBuildListeners().stream()
+                    .filter(o -> o instanceof NbBuildLogger)
+                    .map(o -> (NbBuildLogger)o)
+                    .findFirst()
+                    .orElse(null);
+                if (buildLogger != null) {
+                        tasks.submit(new MarkupCopier(stdout, Project.MSG_INFO, false, outEncoding, buildLogger, foldingHelper));
+                        tasks.submit(new MarkupCopier(stderr, Project.MSG_WARN, true, outEncoding, buildLogger, foldingHelper));
+                } else {
+                    setCopier(stdout, getOutputStream(), delegateOutputStream, false);
+                    setCopier(stderr, getErrorStream(), delegateErrorStream, true);
+                }                                
+                InputStream is = getInputStream();
+                if (is == null)
+                    is = AntBridge.delegateInputStream();
+                inputTask = tasks.submit(new TransferCopier(is, stdin));
+            }
 
             public void stop() {
-                if (errTask != null) {
-                    try {
-                        errTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outTask != null) {
-                    try {
-                        outTask.join(3000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (inTask != null) {
-                    inTask.interrupt();
-                    try {
-                        inTask.join(1000);
-                    } catch (InterruptedException ex) {
-                    }
-                }
-                if (outCopier != null) {
-                    outCopier.maybeFlush();
-                }
-                if (errCopier != null) {
-                    errCopier.maybeFlush();
+                try {
+                    if (inputTask != null)
+                        inputTask.cancel(true);
+                    tasks.shutdown();
+                    tasks.awaitTermination(3, TimeUnit.SECONDS);
+                } catch (InterruptedException ex) {
+                } finally {
+                    tasks.shutdownNow();
                 }
             }
 
             public void setProcessOutputStream(InputStream inputStream) throws IOException {
-                OutputStream os = getOutputStream();
-                Integer logLevel = null;
-                if (os == null || delegateOutputStream) {
-                    os = AntBridge.delegateOutputStream(false);
-                    logLevel = Project.MSG_INFO;
-                }
-                outTask = new Thread(Thread.currentThread().getThreadGroup(), outCopier = new Copier(inputStream, os, logLevel, outEncoding, foldingHelper),
-                        "Out Thread for " + getProject().getName()); // NOI18N
-                outTask.setDaemon(true);
-                outTask.start();
+                this.stdout = inputStream;
             }
 
             public void setProcessErrorStream(InputStream inputStream) throws IOException {
-                OutputStream os = getErrorStream();
-                Integer logLevel = null;
-                if (os == null || delegateErrorStream) {
-                    os = AntBridge.delegateOutputStream(true);
-                    logLevel = Project.MSG_WARN;
-                }
-                errTask = new Thread(Thread.currentThread().getThreadGroup(), errCopier = new Copier(inputStream, os, logLevel, errEncoding, foldingHelper),
-                        "Err Thread for " + getProject().getName()); // NOI18N
-                errTask.setDaemon(true);
-                errTask.start();
+                this.stderr = inputStream;
             }
 
             public void setProcessInputStream(OutputStream outputStream) throws IOException {
-                InputStream is = getInputStream();
-                if (is == null) {
-                    is = AntBridge.delegateInputStream();
-                }
-                inTask = new Thread(Thread.currentThread().getThreadGroup(), new Copier(is, outputStream, null, null, foldingHelper),
-                        "In Thread for " + getProject().getName()); // NOI18N
-                inTask.setDaemon(true);
-                inTask.start();
+                this.stdin = outputStream;
             }
 
         }
 
     }
 
-    private class Copier implements Runnable {
+    /**
+     * Simple copier that transfers all input to output.
+     */
+    private class TransferCopier implements Runnable {
 
         private final InputStream in;
         private final OutputStream out;
-        private final Integer logLevel;
+
+        public TransferCopier(InputStream in, OutputStream out) {
+            this.in = in;
+            this.out = out;
+        }
+
+        @Override
+        public void run() {
+            try {
+                byte[] data = new byte[1024];
+                int len;
+                while ((len = in.read(data)) >= 0) {
+                    out.write(data, 0, len);
+                    out.flush();
+                }
+            } catch (IOException  x) {
+                // ignore IOException: Broken pipe from FileOutputStream.writeBytes in BufferedOutputStream.flush
+            }
+        }
+
+    }
+
+    /**
+     * Filtering copier that marks up links, ignoring stack traces.
+     */
+    private class MarkupCopier implements Runnable {
+
+        private final InputStream in;
+        private final int logLevel;
         private final String encoding;
         private final RequestProcessor.Task flusher;
         private final ByteArrayOutputStream currentLine;
-        private OutputWriter ow = null;
-        private boolean err;
-        private AntSession session = null;
+        private final OutputWriter ow;
+        private final boolean err;
+        private final AntSession session;
         private final FoldingHelper foldingHelper;
 
-        public Copier(InputStream in, OutputStream out, Integer logLevel, String encoding/*, long init*/,
-                FoldingHelper foldingHelper) {
+        public MarkupCopier(InputStream in, int logLevel, boolean err, String encoding, NbBuildLogger buildLogger, FoldingHelper foldingHelper) {
             this.in = in;
-            this.out = out;
             this.logLevel = logLevel;
+            this.err = err;
             this.encoding = encoding;
             this.foldingHelper = foldingHelper;
-            if (logLevel != null) {
-                flusher = PROCESSOR.create(new Runnable() {
-                    public void run() {
-                        maybeFlush();
-                    }
-                });
-                currentLine = new ByteArrayOutputStream();
+
+            flusher = PROCESSOR.create(() -> maybeFlush(false));
+            currentLine = new ByteArrayOutputStream();
+
+            ow = err ? buildLogger.err : buildLogger.out;
+            session = buildLogger.thisSession;
+        }
+
+        private synchronized void append(byte[] data, int off, int len) {
+            currentLine.write(data, off, len);
+            if (currentLine.size() > 8192) {
+                flusher.run();
             } else {
-                flusher = null;
-                currentLine = null;
+                flusher.schedule(250);
             }
         }
 
+        private synchronized String appendAndTake(byte[] data, int off, int len) throws UnsupportedEncodingException {
+            currentLine.write(data, off, len);
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
+        private synchronized String take() throws UnsupportedEncodingException {
+            String str = currentLine.toString(encoding);
+            currentLine.reset();
+            return str;
+        }
+
         public void run() {
-            /*
-            StringBuilder content = new StringBuilder();
-            long tick = System.currentTimeMillis();
-            content.append(String.format("[init: %1.1fsec]", (tick - init) / 1000.0));
-             */
-            
-            if (ow == null && logLevel != null) {
-                Vector v = getProject().getBuildListeners();
-                for (Object o : v) {
-                    if (o instanceof NbBuildLogger) {
-                        NbBuildLogger l = (NbBuildLogger) o;
-                        err = logLevel != Project.MSG_INFO;
-                        ow = err ? l.err : l.out;
-                        session = l.thisSession;
-                        break;
-                    }
-                }
-            }
             try {
+                byte[] data = new byte[1024];
+                int len;
+
                 try {
-                    int c;
-                    while ((c = in.read()) != -1) {
-                        if (logLevel == null) {
-                            // Input gets sent immediately.
-                            out.write(c);
-                            out.flush();
-                        } else {
-                            synchronized (this) {
-                                if (c == '\n') {                                    
-                                    String str = currentLine.toString(encoding);
-                                    int len = str.length();
-                                    if (len > 0 && str.charAt(len - 1) == '\r') {
-                                        str = str.substring(0, len - 1);
-                                    }
+                    while ((len = in.read(data)) >= 0) {
+                        int last = 0;
+                        for (int i = 0; i < len; i++) {
+                            int c = data[i] & 0xff;
 
+                            // Add folds for stack traces and mark up lines
+                            // not processed by JavaAntLogger stack trace detection
+                            if (c == '\n') {
+                                String str = appendAndTake(data, last, i > last && data[i - 1] == '\r' ? i - last - 1 : i - last);
+
+                                synchronized (foldingHelper) {
                                     foldingHelper.checkFolds(str, err, session);
-                                    if (str.length() < LOGGER_MAX_LINE_LENGTH) { // not too long message, probably interesting
-                                        // skip stack traces (hyperlinks are created by JavaAntLogger), everything else write directly
-                                        if (!STACK_TRACE.matcher(str).find()) {
-                                            StandardLogger.findHyperlink(str, session, null).println(session, err);
-                                        }
-                                    } else {
-                                        // do not match long strings, directly create a trivial hyperlink
+                                    if (str.length() >= LOGGER_MAX_LINE_LENGTH || !STACK_TRACE.matcher(str).matches())
                                         StandardLogger.findHyperlink(str, session, null).println(session, err);

Review Comment:
   I'm pretty certain it does because it contains .* on both ends so will always match the whole string anyway.  Also probably more to the point this is just to check if it should ignore the lines because JavaAntLogger will mark them up - and JavaAntLogger uses the same regex and uses matches().



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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] mbien commented on pull request #4180: Improve output window performance for ant java tasks #4141.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4180:
URL: https://github.com/apache/netbeans/pull/4180#issuecomment-1143606374

   > Hi, maybe you should check your name in your git sound like an avatar
   
   ```
   git commit --amend --author="Author Name <em...@address.com>" --no-edit
   ```
   this would update the author of the last commit in the current branch. After that just force push to the remote branch.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
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