[GitHub] ant pull request #58: Use StringBuilder instead of StringBuffer as it offers...

[bodewig]

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

[bodewig]

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

[jaikiran]

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

[jaikiran]

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

[reudismam]

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