[GitHub] commons-rdf pull request #49: Cleanup for FindBugs and PMD warnings in -simp...
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/commons-rdf/pull/49#discussion_r168275645
--- Diff:
commons-rdf-simple/src/main/java/org/apache/commons/rdf/simple/experimental/AbstractRDFParser.java
---
@@ -58,8 +60,12 @@
*/
public abstract class AbstractRDFParser>
implements RDFParser, Cloneable {
-public static final ThreadGroup threadGroup = new ThreadGroup("Commons
RDF parsers");
-private static final ExecutorService threadpool =
Executors.newCachedThreadPool(r -> new Thread(threadGroup, r));
+public static final AtomicInteger threadCount = new AtomicInteger();
+private static Thread newThread(Runnable r) {
--- End diff --
That's why I put [the name "Commons RDF Parser
"](https://github.com/apache/commons-rdf/pull/49/files/f1649e034a2623137434fcc2810f8802d3ee9434#diff-68c2acaf43f1ae22da0bdb4eac55a201R65)
back in. But like I said, I am fine with whatever- I was just tearing through
PMD warnings at speed after the comment (I think to the board report) about
there being a lot of them. Shall I take your comment as a direction to remove
this change? Happy to...
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-rdf pull request #49: Cleanup for FindBugs and PMD warnings in -simp...
Github user stain commented on a diff in the pull request:
https://github.com/apache/commons-rdf/pull/49#discussion_r168274256
--- Diff:
commons-rdf-simple/src/main/java/org/apache/commons/rdf/simple/experimental/AbstractRDFParser.java
---
@@ -58,8 +60,12 @@
*/
public abstract class AbstractRDFParser>
implements RDFParser, Cloneable {
-public static final ThreadGroup threadGroup = new ThreadGroup("Commons
RDF parsers");
-private static final ExecutorService threadpool =
Executors.newCachedThreadPool(r -> new Thread(threadGroup, r));
+public static final AtomicInteger threadCount = new AtomicInteger();
+private static Thread newThread(Runnable r) {
--- End diff --
A final ThreadGroup that is not thread safe.. The whole purpose of it is to
group threads?
It is true that they should not be used for security purpose, but that is
not why it is here. The ThreadGroup is mainly useful for debugging as people
might see these "Commons RDF Parser" tree grouped together in the debugger
(e.g. in Eclipse) rather than the generic no-name you get from the
ExecutorService.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-rdf pull request #49: Cleanup for FindBugs and PMD warnings in -simp...
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/commons-rdf/pull/49#discussion_r168260775
--- Diff:
commons-rdf-simple/src/main/java/org/apache/commons/rdf/simple/experimental/AbstractRDFParser.java
---
@@ -58,8 +60,12 @@
*/
public abstract class AbstractRDFParser>
implements RDFParser, Cloneable {
-public static final ThreadGroup threadGroup = new ThreadGroup("Commons
RDF parsers");
-private static final ExecutorService threadpool =
Executors.newCachedThreadPool(r -> new Thread(threadGroup, r));
+public static final AtomicInteger threadCount = new AtomicInteger();
+private static Thread newThread(Runnable r) {
--- End diff --
Because either FindBugs or PMD (can't remember) which called out
`ThreadGroup` as not threadsafe, which is correct as far as I know. I am in
_no_ way wedded to this change and if we can guarantee thread-safety from
outside the class (or simply document it as not threadsafe) I would be happy to
pull this change out.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-rdf pull request #49: Cleanup for FindBugs and PMD warnings in -simp...
Github user stain commented on a diff in the pull request:
https://github.com/apache/commons-rdf/pull/49#discussion_r167732171
--- Diff:
commons-rdf-simple/src/main/java/org/apache/commons/rdf/simple/experimental/AbstractRDFParser.java
---
@@ -58,8 +60,12 @@
*/
public abstract class AbstractRDFParser>
implements RDFParser, Cloneable {
-public static final ThreadGroup threadGroup = new ThreadGroup("Commons
RDF parsers");
-private static final ExecutorService threadpool =
Executors.newCachedThreadPool(r -> new Thread(threadGroup, r));
+public static final AtomicInteger threadCount = new AtomicInteger();
+private static Thread newThread(Runnable r) {
--- End diff --
I disagree on this style change, the previous code was much cleaner. Why is
a method-reference on 4 lines that is only used once better than a very small
lambda?
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-rdf pull request #49: Cleanup for FindBugs and PMD warnings in -simp...
GitHub user ajs6f opened a pull request: https://github.com/apache/commons-rdf/pull/49 Cleanup for FindBugs and PMD warnings in -simple and -jena You can merge this pull request into a Git repository by running: $ git pull https://github.com/ajs6f/commons-rdf JenaSimpleCleanup Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-rdf/pull/49.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 #49 commit f1649e034a2623137434fcc2810f8802d3ee9434 Author: ajs6f Date: 2017-12-14T17:37:25Z Cleanup for FindBugs and PMD warnings in commons-rdf-simple and commons-rdf-jena --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
