[GitHub] ant pull request #58: Use StringBuilder instead of StringBuffer as it offers...
Github user bodewig commented on a diff in the pull request: https://github.com/apache/ant/pull/58#discussion_r166217627 --- Diff: src/main/org/apache/tools/ant/taskdefs/Parallel.java --- @@ -377,7 +377,7 @@ public synchronized void run() { } // now did any of the threads throw an exception -exceptionMessage = new StringBuffer(); +exceptionMessage = new StringBuilder(); --- End diff -- Looking through the class I don't think `exceptionMessage` needs to be an instance field at all, it could be a local variable in `spinThreads` and get passed as an argument to `processExceptions` without doing any harm. To me it seems it is only ever used by a single thread. Most probably a further refactoring could get rid of the `first*` instance fields as well and have `processExceptions` return all their values nicely encapsulated in a single object - including the accumulated messages. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant pull request #58: Use StringBuilder instead of StringBuffer as it offers...
Github user bodewig commented on a diff in the pull request: https://github.com/apache/ant/pull/58#discussion_r166216151 --- Diff: src/main/org/apache/tools/ant/listener/MailLogger.java --- @@ -102,7 +102,7 @@ private static final String DEFAULT_MIME_TYPE = "text/plain"; /** Buffer in which the message is constructed prior to sending */ -private StringBuffer buffer = new StringBuffer(); +private StringBuilder buffer = new StringBuilder(); --- End diff -- Given the `parallel` task and things like our execute framework that spawns new threads, loggers need to be thread safe, But to be honest I don't think we've ever verified `MailLogger` actually is. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant pull request #58: Use StringBuilder instead of StringBuffer as it offers...
Github user jaikiran commented on a diff in the pull request: https://github.com/apache/ant/pull/58#discussion_r166183671 --- Diff: src/main/org/apache/tools/ant/taskdefs/Parallel.java --- @@ -377,7 +377,7 @@ public synchronized void run() { } // now did any of the threads throw an exception -exceptionMessage = new StringBuffer(); +exceptionMessage = new StringBuilder(); --- End diff -- Same comment as above. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant pull request #58: Use StringBuilder instead of StringBuffer as it offers...
Github user jaikiran commented on a diff in the pull request: https://github.com/apache/ant/pull/58#discussion_r166183611 --- Diff: src/main/org/apache/tools/ant/listener/MailLogger.java --- @@ -102,7 +102,7 @@ private static final String DEFAULT_MIME_TYPE = "text/plain"; /** Buffer in which the message is constructed prior to sending */ -private StringBuffer buffer = new StringBuffer(); +private StringBuilder buffer = new StringBuilder(); --- End diff -- I'm not too sure this change here, in this class is correct. The `StringBuffer` is a thread-safe class whereas the `StringBuilder` isn't. Having said that I haven't checked yet whether MailLogger class is expected to be thread safe nor have I checked its usage thoroughly. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant pull request #58: Use StringBuilder instead of StringBuffer as it offers...
GitHub user reudismam opened a pull request: https://github.com/apache/ant/pull/58 Use StringBuilder instead of StringBuffer as it offers high performan⦠â¦ce in single thread places as it is generally the case. You can merge this pull request into a Git repository by running: $ git pull https://github.com/reudismam/ant builder Alternatively you can review and apply these changes as the patch at: https://github.com/apache/ant/pull/58.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #58 commit bf3d0b448ed055eea7dcc036397e8b0f7cbc50fe Author: reudismam Date: 2018-02-05T16:18:05Z Use StringBuilder instead of StringBuffer as it offers high performance in single thread places as it is generally the case. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org