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 2023/01/18 20:36:33 UTC
[GitHub] [netbeans] lbownik opened a new pull request, #5317: code cleanup - no logic changes
lbownik opened a new pull request, #5317:
URL: https://github.com/apache/netbeans/pull/5317
added @Override
used diamonds
fixes some "raw type" compilation warnings
replaced some anonymous classes with lambdas
replaced iterators with for-each loops or streams
replaced array iterations with streams
removed unused private util methods
removed commented out code
---
**^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.
If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1082806548
##########
harness/o.n.insane/src/org/netbeans/insane/model/BinaryHeapModel.java:
##########
@@ -49,7 +49,7 @@ public static HeapModel open(File file) throws Exception {
// mmap it
long len = data.length();
buffer = new FileInputStream(data).getChannel().map(FileChannel.MapMode.READ_ONLY, 0, len);
- System.err.println("magic=" + buffer.getInt(0));
+ System.err.println("magic=" + buffer.getInt(0)); // shouldn't this be replaced with logging statement?
Review Comment:
sure - comment removed in https://github.com/apache/netbeans/pull/5317/commits/631e3a89503ae7112dc75357a8625ac2ea51f528
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081231463
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/SimpleXmlVisitor.java:
##########
@@ -61,7 +61,7 @@ public void close() throws IOException {
// ignore for this xml format
- public void visitClass(Class cls) {}
+ public void visitClass(Class<?> cls) {}
Review Comment:
reverted
--
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] lbownik commented on pull request #5317: code cleanup - no logic changes
Posted by "lbownik (via GitHub)" <gi...@apache.org>.
lbownik commented on PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#issuecomment-1407380212
@viero - tests are weak, but you gyus cosider writing tests for already existing code a noise :\
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081259499
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:
##########
@@ -233,58 +231,22 @@ public void run() {
if (ret[0] != null) throw ret[0];
}
- private static Thread[] getAllThreads() {
- ThreadGroup act = Thread.currentThread().getThreadGroup();
- while (act.getParent() != null) act = act.getParent();
- Thread[] all = new Thread[2*act.activeCount()+5];
- int cnt = act.enumerate(all);
- Thread[] ret = new Thread[cnt];
- System.arraycopy(all, 0, ret, 0, cnt);
- return ret;
- }
-
- @SuppressWarnings("deprecation")
- private static void suspendAllThreads(Set<Thread> except) {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- if (!except.contains(threads[i])) {
- if ((threads[i].getName().indexOf("VM") == -1) &&
- (threads[i].getName().indexOf("CompilerTh") == -1) &&
- (threads[i].getName().indexOf("Signal") == -1)) {
- System.out.println("suspending " + threads[i]);
- threads[i].suspend();
- }
- }
- }
- }
-
- @SuppressWarnings("deprecation")
- private static void resumeAllThreads() {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- threads[i].resume();
- }
- }
-
private static class ClassInfo {
- private static Map<Class, ClassInfo> classInfoRegistry = new WeakHashMap<Class, ClassInfo>();
- private int size;
+ private static final Map<Class<?>, ClassInfo> classInfoRegistry = new WeakHashMap<>();
+ private final int size;
- private ClassInfo(Class cls) {
-// this.cls = cls;
+ private ClassInfo(Class<?> cls) {
if (cls.isArray()) {
- Class base = cls.getComponentType();
+ Class<?> base = cls.getComponentType();
size = -getSize(base, true);
} else {
// iterate all fields and sum the sizes
int sum = 0;
- for (Class act = cls; act != null; act = act.getSuperclass()) {
- Field[] flds = act.getDeclaredFields();
- for (int i=0; i<flds.length; i++) { // count all nonstatic fields
- if ((flds[i].getModifiers() & Modifier.STATIC) == 0) {
- sum += getSize(flds[i].getType(), false);
+ for (Class<?> act = cls; act != null; act = act.getSuperclass()) {
+ // count all nonstatic fields
Review Comment:
I haven't modified the logic, just converted indexed loop with for-each loop
sum is used in line 291
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081240128
##########
harness/o.n.insane/src/org/netbeans/insane/impl/LiveEngine.java:
##########
@@ -98,7 +105,8 @@ private void visitRef(Object from, Object to, Field field) {
addIncommingRef(to, from, field);
if (rest.containsKey(to)) {
rest.remove(to);
- if (rest.size() == 0) throw new ObjectFoundException();
+ if (rest.isEmpty())
+ throw new ObjectFoundException();
Review Comment:
added
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081218008
##########
harness/o.n.insane/src/org/netbeans/insane/model/InsaneConverter.java:
##########
@@ -351,8 +351,8 @@ private void compute() {
objsOffset = currentOffset;
// compute offsets of instances
- for (Iterator<InstanceInfo> it = instanceInfo.iterator(); it.hasNext(); ) {
- InstanceInfo info = it.next();
+ for (Iterator it = instanceInfo.iterator(); it.hasNext(); ) {
+ InstanceInfo info = (InstanceInfo)it.next();
Review Comment:
it fixes compilation error
--
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] matthiasblaesing commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081846702
##########
harness/o.n.insane/src/org/netbeans/insane/model/InsaneConverter.java:
##########
@@ -351,8 +351,8 @@ private void compute() {
objsOffset = currentOffset;
// compute offsets of instances
- for (Iterator<InstanceInfo> it = instanceInfo.iterator(); it.hasNext(); ) {
- InstanceInfo info = it.next();
+ for (Iterator it = instanceInfo.iterator(); it.hasNext(); ) {
+ InstanceInfo info = (InstanceInfo)it.next();
Review Comment:
The compilation error is caused by using the wrong generics in `ObjectSet`, you reverted the introduced generic. Exactly these kind of problems are the reason to be very careful to quickly introduce generics, there is a high chance, that it will be not the right type.
##########
harness/o.n.insane/src/org/netbeans/insane/model/BinaryHeapModel.java:
##########
@@ -49,7 +49,7 @@ public static HeapModel open(File file) throws Exception {
// mmap it
long len = data.length();
buffer = new FileInputStream(data).getChannel().map(FileChannel.MapMode.READ_ONLY, 0, len);
- System.err.println("magic=" + buffer.getInt(0));
+ System.err.println("magic=" + buffer.getInt(0)); // shouldn't this be replaced with logging statement?
Review Comment:
Please don't add questions - it is useless.
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:
##########
@@ -233,58 +231,22 @@ public void run() {
if (ret[0] != null) throw ret[0];
}
- private static Thread[] getAllThreads() {
- ThreadGroup act = Thread.currentThread().getThreadGroup();
- while (act.getParent() != null) act = act.getParent();
- Thread[] all = new Thread[2*act.activeCount()+5];
- int cnt = act.enumerate(all);
- Thread[] ret = new Thread[cnt];
- System.arraycopy(all, 0, ret, 0, cnt);
- return ret;
- }
-
- @SuppressWarnings("deprecation")
- private static void suspendAllThreads(Set<Thread> except) {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- if (!except.contains(threads[i])) {
- if ((threads[i].getName().indexOf("VM") == -1) &&
- (threads[i].getName().indexOf("CompilerTh") == -1) &&
- (threads[i].getName().indexOf("Signal") == -1)) {
- System.out.println("suspending " + threads[i]);
- threads[i].suspend();
- }
- }
- }
- }
-
- @SuppressWarnings("deprecation")
- private static void resumeAllThreads() {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- threads[i].resume();
- }
- }
-
private static class ClassInfo {
- private static Map<Class, ClassInfo> classInfoRegistry = new WeakHashMap<Class, ClassInfo>();
- private int size;
+ private static final Map<Class<?>, ClassInfo> classInfoRegistry = new WeakHashMap<>();
+ private final int size;
- private ClassInfo(Class cls) {
-// this.cls = cls;
+ private ClassInfo(Class<?> cls) {
if (cls.isArray()) {
- Class base = cls.getComponentType();
+ Class<?> base = cls.getComponentType();
size = -getSize(base, true);
} else {
// iterate all fields and sum the sizes
int sum = 0;
- for (Class act = cls; act != null; act = act.getSuperclass()) {
- Field[] flds = act.getDeclaredFields();
- for (int i=0; i<flds.length; i++) { // count all nonstatic fields
- if ((flds[i].getModifiers() & Modifier.STATIC) == 0) {
- sum += getSize(flds[i].getType(), false);
+ for (Class<?> act = cls; act != null; act = act.getSuperclass()) {
+ // count all nonstatic fields
Review Comment:
The comment in line 246 did not exist before, the variable `sum` is self-explanatory and the comment in line 243 gives all context, that is needed. This new comment is just wrong. The loop does not count, it sums.
--
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] lbownik closed pull request #5317: harness/o.n.insane code cleanup
Posted by "lbownik (via GitHub)" <gi...@apache.org>.
lbownik closed pull request #5317: harness/o.n.insane code cleanup
URL: https://github.com/apache/netbeans/pull/5317
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081239885
##########
harness/o.n.insane/src/org/netbeans/insane/model/ObjectSet.java:
##########
@@ -90,7 +90,7 @@ public boolean contains(Object key) {
return get(key) != null;
}
- public Iterator iterator() {
+ public Iterator<Object> iterator() {
return new Iterator() {
Review Comment:
reverted
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081221565
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:
##########
@@ -210,19 +213,14 @@ public static Set<Object> interestingRoots() {
* @param v a Visitor to be notified on all found objects and references.
* @param roots a Collection of objects to be evaluated.
*/
- public static void scanExclusivelyInAWT(final Filter f, final Visitor v, final Set roots) throws Exception {
+ public static void scanExclusivelyInAWT(final Filter f, final Visitor v, final Set<?> roots) throws Exception {
Review Comment:
reverted
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1082807213
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:
##########
@@ -233,58 +231,22 @@ public void run() {
if (ret[0] != null) throw ret[0];
}
- private static Thread[] getAllThreads() {
- ThreadGroup act = Thread.currentThread().getThreadGroup();
- while (act.getParent() != null) act = act.getParent();
- Thread[] all = new Thread[2*act.activeCount()+5];
- int cnt = act.enumerate(all);
- Thread[] ret = new Thread[cnt];
- System.arraycopy(all, 0, ret, 0, cnt);
- return ret;
- }
-
- @SuppressWarnings("deprecation")
- private static void suspendAllThreads(Set<Thread> except) {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- if (!except.contains(threads[i])) {
- if ((threads[i].getName().indexOf("VM") == -1) &&
- (threads[i].getName().indexOf("CompilerTh") == -1) &&
- (threads[i].getName().indexOf("Signal") == -1)) {
- System.out.println("suspending " + threads[i]);
- threads[i].suspend();
- }
- }
- }
- }
-
- @SuppressWarnings("deprecation")
- private static void resumeAllThreads() {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- threads[i].resume();
- }
- }
-
private static class ClassInfo {
- private static Map<Class, ClassInfo> classInfoRegistry = new WeakHashMap<Class, ClassInfo>();
- private int size;
+ private static final Map<Class<?>, ClassInfo> classInfoRegistry = new WeakHashMap<>();
+ private final int size;
- private ClassInfo(Class cls) {
-// this.cls = cls;
+ private ClassInfo(Class<?> cls) {
if (cls.isArray()) {
- Class base = cls.getComponentType();
+ Class<?> base = cls.getComponentType();
size = -getSize(base, true);
} else {
// iterate all fields and sum the sizes
int sum = 0;
- for (Class act = cls; act != null; act = act.getSuperclass()) {
- Field[] flds = act.getDeclaredFields();
- for (int i=0; i<flds.length; i++) { // count all nonstatic fields
- if ((flds[i].getModifiers() & Modifier.STATIC) == 0) {
- sum += getSize(flds[i].getType(), false);
+ for (Class<?> act = cls; act != null; act = act.getSuperclass()) {
+ // count all nonstatic fields
Review Comment:
comment removed in https://github.com/apache/netbeans/pull/5317/commits/631e3a89503ae7112dc75357a8625ac2ea51f528
--
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] vieiro commented on pull request #5317: code cleanup - no logic changes
Posted by "vieiro (via GitHub)" <gi...@apache.org>.
vieiro commented on PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#issuecomment-1407443158
> @vieiro - Yes. I will write tests. Meanwhile mark this PR as "do-not-merge" or something. An one more thing.
Good news! We'll wait for those tests them. Remember that we prefer _new_ tests, without modifying existing ones. The idea here is that new tests can replace existing ones later on, if needed. That will be easier to review for us.
> The module does not mention any public packages in https://github.com/apache/netbeans/blob/master/harness/o.n.insane/nbproject/project.xml does it mean that it does not expose any APIs to other molecules or that every public class in all packages is exposed?
Not all modules expose APIs to other modules, only if they have something interesting to expose to the world. This one exposes nothing.
--
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] vieiro commented on pull request #5317: code cleanup - no logic changes
Posted by "vieiro (via GitHub)" <gi...@apache.org>.
vieiro commented on PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#issuecomment-1407395175
> @viero - tests are weak, but you gyus cosider writing tests for already existing code a noise :\
On the contrary! I may understand people getting concerned if you _modify_ existing testing code, but _adding_ new tests for existing code is always welcome. Do you want to take a go at writing tests for this module in another PR? This may help us verifying this module works as expected and no subtle new bugs are introduced.
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1082806548
##########
harness/o.n.insane/src/org/netbeans/insane/model/BinaryHeapModel.java:
##########
@@ -49,7 +49,7 @@ public static HeapModel open(File file) throws Exception {
// mmap it
long len = data.length();
buffer = new FileInputStream(data).getChannel().map(FileChannel.MapMode.READ_ONLY, 0, len);
- System.err.println("magic=" + buffer.getInt(0));
+ System.err.println("magic=" + buffer.getInt(0)); // shouldn't this be replaced with logging statement?
Review Comment:
sure
--
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] lbownik commented on pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#issuecomment-1397056532
> > > i don't quite see the point in converting for loops over arrays to streams.. just to loop.
> > > for loops are perfectly fine for looping.
> >
> >
> > One line of code instead of two. Shall I rewrite them to for-each loops?
>
> for-each loops would be better here yes. Also please try to use CI resources responsibly. Don't push every individual change into a PR. Every PR sync does several hours of testing on many nodes. All apache projects share the same cluster.
Ok. I'll rewrite and try to save CI resources.
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081239466
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/CountingVisitor.java:
##########
@@ -59,22 +61,25 @@ public void visitObject(ObjectMap map, Object obj) {
size += objSize;
}
+ @Override
public void visitStaticReference(ObjectMap map, Object to, java.lang.reflect.Field ref) {}
+ @Override
public void visitObjectReference(ObjectMap map, Object from, Object to, java.lang.reflect.Field ref) {}
+ @Override
public void visitArrayReference(ObjectMap map, Object from, Object to, int index) {}
public Set<Class<?>> getClasses() {
return Collections.unmodifiableSet(infoMap.keySet());
}
- public int getCountForClass(Class cls) {
+ public int getCountForClass(Class<?> cls) {
Review Comment:
reverted
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1082399583
##########
harness/o.n.insane/src/org/netbeans/insane/model/BinaryHeapModel.java:
##########
@@ -49,7 +49,7 @@ public static HeapModel open(File file) throws Exception {
// mmap it
long len = data.length();
buffer = new FileInputStream(data).getChannel().map(FileChannel.MapMode.READ_ONLY, 0, len);
- System.err.println("magic=" + buffer.getInt(0));
+ System.err.println("magic=" + buffer.getInt(0)); // shouldn't this be replaced with logging statement?
Review Comment:
ha - but you spotted it and this was my intention :))))
the thing is that the whole module does not use Loggers but prints to System.err - do You think it is a good oppotunity to employ loggin through Loggger?
I'll remove this comment in the next commit enyway.
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081221565
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:
##########
@@ -210,19 +213,14 @@ public static Set<Object> interestingRoots() {
* @param v a Visitor to be notified on all found objects and references.
* @param roots a Collection of objects to be evaluated.
*/
- public static void scanExclusivelyInAWT(final Filter f, final Visitor v, final Set roots) throws Exception {
+ public static void scanExclusivelyInAWT(final Filter f, final Visitor v, final Set<?> roots) throws Exception {
Review Comment:
the package is not listed in <public-packages>
also grepping for ScannerUtils.scanExclusivelyInAWT shows no results.
this function seems unused - maybe should be deleted?
--
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] lbownik commented on pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#issuecomment-1396984048
> i don't quite see the point in converting for loops over arrays to streams.. just to loop.
>
> for loops are perfectly fine for looping.
One line of code instead of two.
Shall I rewrite them to for-each loops?
--
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] matthiasblaesing commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1082738982
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:
##########
@@ -233,58 +231,22 @@ public void run() {
if (ret[0] != null) throw ret[0];
}
- private static Thread[] getAllThreads() {
- ThreadGroup act = Thread.currentThread().getThreadGroup();
- while (act.getParent() != null) act = act.getParent();
- Thread[] all = new Thread[2*act.activeCount()+5];
- int cnt = act.enumerate(all);
- Thread[] ret = new Thread[cnt];
- System.arraycopy(all, 0, ret, 0, cnt);
- return ret;
- }
-
- @SuppressWarnings("deprecation")
- private static void suspendAllThreads(Set<Thread> except) {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- if (!except.contains(threads[i])) {
- if ((threads[i].getName().indexOf("VM") == -1) &&
- (threads[i].getName().indexOf("CompilerTh") == -1) &&
- (threads[i].getName().indexOf("Signal") == -1)) {
- System.out.println("suspending " + threads[i]);
- threads[i].suspend();
- }
- }
- }
- }
-
- @SuppressWarnings("deprecation")
- private static void resumeAllThreads() {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- threads[i].resume();
- }
- }
-
private static class ClassInfo {
- private static Map<Class, ClassInfo> classInfoRegistry = new WeakHashMap<Class, ClassInfo>();
- private int size;
+ private static final Map<Class<?>, ClassInfo> classInfoRegistry = new WeakHashMap<>();
+ private final int size;
- private ClassInfo(Class cls) {
-// this.cls = cls;
+ private ClassInfo(Class<?> cls) {
if (cls.isArray()) {
- Class base = cls.getComponentType();
+ Class<?> base = cls.getComponentType();
size = -getSize(base, true);
} else {
// iterate all fields and sum the sizes
int sum = 0;
- for (Class act = cls; act != null; act = act.getSuperclass()) {
- Field[] flds = act.getDeclaredFields();
- for (int i=0; i<flds.length; i++) { // count all nonstatic fields
- if ((flds[i].getModifiers() & Modifier.STATIC) == 0) {
- sum += getSize(flds[i].getType(), false);
+ for (Class<?> act = cls; act != null; act = act.getSuperclass()) {
+ // count all nonstatic fields
Review Comment:
Ah I finally saw it. I never expected a comment on such a trivial loop (I stopped reading at the opening brace). The author wanted to say "iterate" not "count". I would remove it or change new line 243 to read "iterate all nonstatic fields and sum the sizes". Then it should be correct.
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081237658
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:
##########
@@ -169,7 +172,7 @@ public static int sizeOf(Object o) {
* @return the size of all objects in the filtered transitive closure
* of the roots collection.
*/
- public static int recursiveSizeOf(Collection roots, Filter f) throws Exception {
+ public static int recursiveSizeOf(Collection<?> roots, Filter f) throws Exception {
Review Comment:
reverted
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081244062
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/CountingVisitor.java:
##########
@@ -59,22 +61,25 @@ public void visitObject(ObjectMap map, Object obj) {
size += objSize;
}
+ @Override
public void visitStaticReference(ObjectMap map, Object to, java.lang.reflect.Field ref) {}
+ @Override
public void visitObjectReference(ObjectMap map, Object from, Object to, java.lang.reflect.Field ref) {}
+ @Override
public void visitArrayReference(ObjectMap map, Object from, Object to, int index) {}
public Set<Class<?>> getClasses() {
return Collections.unmodifiableSet(infoMap.keySet());
}
- public int getCountForClass(Class cls) {
+ public int getCountForClass(Class<?> cls) {
Info info = infoMap.get(cls);
if (info == null) throw new IllegalArgumentException("Unknown class");
return info.count;
}
- public int getSizeForClass(Class cls) {
+ public int getSizeForClass(Class<?> cls) {
Review Comment:
reverted
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081241776
##########
harness/o.n.insane/src/org/netbeans/insane/impl/InsaneEngine.java:
##########
@@ -51,23 +52,23 @@ public InsaneEngine(Filter f, Visitor v, boolean analyzeStatic) {
public InsaneEngine(ObjectMap backend, Filter f, Visitor v, boolean analyzeStatic) {
objects = backend;
- filter = f == null ? ScannerUtils.noFilter() : f;
+ filter = (f != null ? f : ScannerUtils.noFilter());
Review Comment:
reverted
--
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 #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1082408930
##########
harness/o.n.insane/src/org/netbeans/insane/model/BinaryHeapModel.java:
##########
@@ -49,7 +49,7 @@ public static HeapModel open(File file) throws Exception {
// mmap it
long len = data.length();
buffer = new FileInputStream(data).getChannel().map(FileChannel.MapMode.READ_ONLY, 0, len);
- System.err.println("magic=" + buffer.getInt(0));
+ System.err.println("magic=" + buffer.getInt(0)); // shouldn't this be replaced with logging statement?
Review Comment:
completely changing the logging mechanism of a module is out of scope of PRs which do simple language level cleanup. The title says no logic changes.
If you want to annotate your code in temporary manner, use the github review feature. This feature was made for this.
--
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] vieiro commented on pull request #5317: harness/o.n.insane code cleanup
Posted by "vieiro (via GitHub)" <gi...@apache.org>.
vieiro commented on PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#issuecomment-1407627517
> This module's properties mentions "Export packages onlly to friends" and mentions 3 packages. Does it mean that it is only used by these three mentioned packages? If so does it make sense to test only API calls that are actually used by those packages and maybe later remove all the unused code (code not covered by new tests)?
I'd add whatever tests are required so that we are sure that this problem that Matthias detected...
> The compilation error is caused by using the wrong generics in ObjectSet, you reverted the introduced generic. Exactly these kind of problems are the reason to be very careful to quickly introduce generics, there is a high chance, that it will be not the right type.
... does not happen (nor any other problem).
The objective is to make sure NetBeans works the same (or better) as before the change: i.e., that no bugs are being introduced with the change. Maintainers can detect many bugs, but are humans also and can skip subtle ones.
--
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] matthiasblaesing commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1080572328
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/SimpleXmlVisitor.java:
##########
@@ -61,7 +61,7 @@ public void close() throws IOException {
// ignore for this xml format
- public void visitClass(Class cls) {}
+ public void visitClass(Class<?> cls) {}
Review Comment:
Please don't introduce generics into exported methods in a pure cleanup PR.
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:
##########
@@ -169,7 +172,7 @@ public static int sizeOf(Object o) {
* @return the size of all objects in the filtered transitive closure
* of the roots collection.
*/
- public static int recursiveSizeOf(Collection roots, Filter f) throws Exception {
+ public static int recursiveSizeOf(Collection<?> roots, Filter f) throws Exception {
Review Comment:
If this is exported, please don't add generics for general cleanup.
##########
harness/o.n.insane/src/org/netbeans/insane/model/ObjectSet.java:
##########
@@ -90,7 +90,7 @@ public boolean contains(Object key) {
return get(key) != null;
}
- public Iterator iterator() {
+ public Iterator<Object> iterator() {
return new Iterator() {
Review Comment:
Isn't this missing the generic here?
##########
harness/o.n.insane/src/org/netbeans/insane/impl/InsaneEngine.java:
##########
@@ -51,23 +52,23 @@ public InsaneEngine(Filter f, Visitor v, boolean analyzeStatic) {
public InsaneEngine(ObjectMap backend, Filter f, Visitor v, boolean analyzeStatic) {
objects = backend;
- filter = f == null ? ScannerUtils.noFilter() : f;
+ filter = (f != null ? f : ScannerUtils.noFilter());
visitor = v;
analyzeStaticData = analyzeStatic;
}
// normal set is enough, java.lang.Class have identity-based equals()
- private Set<Class> knownClasses = new HashSet<Class>();
+ private final Set<Class<?>> knownClasses = new HashSet<>();
// The queue for BFS scan of the heap.
// each thing, before added to the queue, is added
// to the known* structures and reported to the visitor
- private Queue<Object> queue = new Queue<Object>();
+ private final Queue<Object> queue = new Queue<>();
- public void traverse(Collection roots) throws Exception {
+ public void traverse(Collection<?> roots) throws Exception {
Review Comment:
If this is exported, please don't add generics for general cleanup.
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:
##########
@@ -233,58 +231,22 @@ public void run() {
if (ret[0] != null) throw ret[0];
}
- private static Thread[] getAllThreads() {
- ThreadGroup act = Thread.currentThread().getThreadGroup();
- while (act.getParent() != null) act = act.getParent();
- Thread[] all = new Thread[2*act.activeCount()+5];
- int cnt = act.enumerate(all);
- Thread[] ret = new Thread[cnt];
- System.arraycopy(all, 0, ret, 0, cnt);
- return ret;
- }
-
- @SuppressWarnings("deprecation")
- private static void suspendAllThreads(Set<Thread> except) {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- if (!except.contains(threads[i])) {
- if ((threads[i].getName().indexOf("VM") == -1) &&
- (threads[i].getName().indexOf("CompilerTh") == -1) &&
- (threads[i].getName().indexOf("Signal") == -1)) {
- System.out.println("suspending " + threads[i]);
- threads[i].suspend();
- }
- }
- }
- }
-
- @SuppressWarnings("deprecation")
- private static void resumeAllThreads() {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- threads[i].resume();
- }
- }
-
private static class ClassInfo {
- private static Map<Class, ClassInfo> classInfoRegistry = new WeakHashMap<Class, ClassInfo>();
- private int size;
+ private static final Map<Class<?>, ClassInfo> classInfoRegistry = new WeakHashMap<>();
+ private final int size;
- private ClassInfo(Class cls) {
-// this.cls = cls;
+ private ClassInfo(Class<?> cls) {
if (cls.isArray()) {
- Class base = cls.getComponentType();
+ Class<?> base = cls.getComponentType();
size = -getSize(base, true);
} else {
// iterate all fields and sum the sizes
int sum = 0;
- for (Class act = cls; act != null; act = act.getSuperclass()) {
- Field[] flds = act.getDeclaredFields();
- for (int i=0; i<flds.length; i++) { // count all nonstatic fields
- if ((flds[i].getModifiers() & Modifier.STATIC) == 0) {
- sum += getSize(flds[i].getType(), false);
+ for (Class<?> act = cls; act != null; act = act.getSuperclass()) {
+ // count all nonstatic fields
Review Comment:
this is not correct, the comment in line 281 says it: It created the sum, not the count. In this case comment is duplicated and the name of the variable `sum` is already a dead give away.
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/CountingVisitor.java:
##########
@@ -59,22 +61,25 @@ public void visitObject(ObjectMap map, Object obj) {
size += objSize;
}
+ @Override
public void visitStaticReference(ObjectMap map, Object to, java.lang.reflect.Field ref) {}
+ @Override
public void visitObjectReference(ObjectMap map, Object from, Object to, java.lang.reflect.Field ref) {}
+ @Override
public void visitArrayReference(ObjectMap map, Object from, Object to, int index) {}
public Set<Class<?>> getClasses() {
return Collections.unmodifiableSet(infoMap.keySet());
}
- public int getCountForClass(Class cls) {
+ public int getCountForClass(Class<?> cls) {
Review Comment:
If this is exported, please don't add generics for general cleanup.
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:
##########
@@ -210,19 +213,14 @@ public static Set<Object> interestingRoots() {
* @param v a Visitor to be notified on all found objects and references.
* @param roots a Collection of objects to be evaluated.
*/
- public static void scanExclusivelyInAWT(final Filter f, final Visitor v, final Set roots) throws Exception {
+ public static void scanExclusivelyInAWT(final Filter f, final Visitor v, final Set<?> roots) throws Exception {
Review Comment:
If this is exported, please don't add generics for general cleanup.
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/CountingVisitor.java:
##########
@@ -59,22 +61,25 @@ public void visitObject(ObjectMap map, Object obj) {
size += objSize;
}
+ @Override
public void visitStaticReference(ObjectMap map, Object to, java.lang.reflect.Field ref) {}
+ @Override
public void visitObjectReference(ObjectMap map, Object from, Object to, java.lang.reflect.Field ref) {}
+ @Override
public void visitArrayReference(ObjectMap map, Object from, Object to, int index) {}
public Set<Class<?>> getClasses() {
return Collections.unmodifiableSet(infoMap.keySet());
}
- public int getCountForClass(Class cls) {
+ public int getCountForClass(Class<?> cls) {
Info info = infoMap.get(cls);
if (info == null) throw new IllegalArgumentException("Unknown class");
return info.count;
}
- public int getSizeForClass(Class cls) {
+ public int getSizeForClass(Class<?> cls) {
Review Comment:
If this is exported, please don't add generics for general cleanup.
##########
harness/o.n.insane/src/org/netbeans/insane/impl/LiveEngine.java:
##########
@@ -98,7 +105,8 @@ private void visitRef(Object from, Object to, Field field) {
addIncommingRef(to, from, field);
if (rest.containsKey(to)) {
rest.remove(to);
- if (rest.size() == 0) throw new ObjectFoundException();
+ if (rest.isEmpty())
+ throw new ObjectFoundException();
Review Comment:
Please add braces
##########
harness/o.n.insane/src/org/netbeans/insane/model/InsaneConverter.java:
##########
@@ -351,8 +351,8 @@ private void compute() {
objsOffset = currentOffset;
// compute offsets of instances
- for (Iterator<InstanceInfo> it = instanceInfo.iterator(); it.hasNext(); ) {
- InstanceInfo info = it.next();
+ for (Iterator it = instanceInfo.iterator(); it.hasNext(); ) {
+ InstanceInfo info = (InstanceInfo)it.next();
Review Comment:
How does this help?
##########
harness/o.n.insane/src/org/netbeans/insane/impl/InsaneEngine.java:
##########
@@ -51,23 +52,23 @@ public InsaneEngine(Filter f, Visitor v, boolean analyzeStatic) {
public InsaneEngine(ObjectMap backend, Filter f, Visitor v, boolean analyzeStatic) {
objects = backend;
- filter = f == null ? ScannerUtils.noFilter() : f;
+ filter = (f != null ? f : ScannerUtils.noFilter());
Review Comment:
I don't see the improvement, on the contrary, the new form introduces noise.
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081240861
##########
harness/o.n.insane/src/org/netbeans/insane/impl/InsaneEngine.java:
##########
@@ -51,23 +52,23 @@ public InsaneEngine(Filter f, Visitor v, boolean analyzeStatic) {
public InsaneEngine(ObjectMap backend, Filter f, Visitor v, boolean analyzeStatic) {
objects = backend;
- filter = f == null ? ScannerUtils.noFilter() : f;
+ filter = (f != null ? f : ScannerUtils.noFilter());
visitor = v;
analyzeStaticData = analyzeStatic;
}
// normal set is enough, java.lang.Class have identity-based equals()
- private Set<Class> knownClasses = new HashSet<Class>();
+ private final Set<Class<?>> knownClasses = new HashSet<>();
// The queue for BFS scan of the heap.
// each thing, before added to the queue, is added
// to the known* structures and reported to the visitor
- private Queue<Object> queue = new Queue<Object>();
+ private final Queue<Object> queue = new Queue<>();
- public void traverse(Collection roots) throws Exception {
+ public void traverse(Collection<?> roots) throws Exception {
Review Comment:
reverted
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1081259499
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:
##########
@@ -233,58 +231,22 @@ public void run() {
if (ret[0] != null) throw ret[0];
}
- private static Thread[] getAllThreads() {
- ThreadGroup act = Thread.currentThread().getThreadGroup();
- while (act.getParent() != null) act = act.getParent();
- Thread[] all = new Thread[2*act.activeCount()+5];
- int cnt = act.enumerate(all);
- Thread[] ret = new Thread[cnt];
- System.arraycopy(all, 0, ret, 0, cnt);
- return ret;
- }
-
- @SuppressWarnings("deprecation")
- private static void suspendAllThreads(Set<Thread> except) {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- if (!except.contains(threads[i])) {
- if ((threads[i].getName().indexOf("VM") == -1) &&
- (threads[i].getName().indexOf("CompilerTh") == -1) &&
- (threads[i].getName().indexOf("Signal") == -1)) {
- System.out.println("suspending " + threads[i]);
- threads[i].suspend();
- }
- }
- }
- }
-
- @SuppressWarnings("deprecation")
- private static void resumeAllThreads() {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- threads[i].resume();
- }
- }
-
private static class ClassInfo {
- private static Map<Class, ClassInfo> classInfoRegistry = new WeakHashMap<Class, ClassInfo>();
- private int size;
+ private static final Map<Class<?>, ClassInfo> classInfoRegistry = new WeakHashMap<>();
+ private final int size;
- private ClassInfo(Class cls) {
-// this.cls = cls;
+ private ClassInfo(Class<?> cls) {
if (cls.isArray()) {
- Class base = cls.getComponentType();
+ Class<?> base = cls.getComponentType();
size = -getSize(base, true);
} else {
// iterate all fields and sum the sizes
int sum = 0;
- for (Class act = cls; act != null; act = act.getSuperclass()) {
- Field[] flds = act.getDeclaredFields();
- for (int i=0; i<flds.length; i++) { // count all nonstatic fields
- if ((flds[i].getModifiers() & Modifier.STATIC) == 0) {
- sum += getSize(flds[i].getType(), false);
+ for (Class<?> act = cls; act != null; act = act.getSuperclass()) {
+ // count all nonstatic fields
Review Comment:
I haven't modified the logic, just converted indexed loop with for-each loop
--
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 #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
mbien commented on PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#issuecomment-1397023312
> > i don't quite see the point in converting for loops over arrays to streams.. just to loop.
> > for loops are perfectly fine for looping.
>
> One line of code instead of two. Shall I rewrite them to for-each loops?
for-each loops would be better here yes. Also please try to use CI resources responsibly. Don't push every individual change into a PR. Every CI does several hours of testing on many nodes. All apache projects share the same cluster.
--
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] lbownik commented on a diff in pull request #5317: code cleanup - no logic changes
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#discussion_r1082397254
##########
harness/o.n.insane/src/org/netbeans/insane/scanner/ScannerUtils.java:
##########
@@ -233,58 +231,22 @@ public void run() {
if (ret[0] != null) throw ret[0];
}
- private static Thread[] getAllThreads() {
- ThreadGroup act = Thread.currentThread().getThreadGroup();
- while (act.getParent() != null) act = act.getParent();
- Thread[] all = new Thread[2*act.activeCount()+5];
- int cnt = act.enumerate(all);
- Thread[] ret = new Thread[cnt];
- System.arraycopy(all, 0, ret, 0, cnt);
- return ret;
- }
-
- @SuppressWarnings("deprecation")
- private static void suspendAllThreads(Set<Thread> except) {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- if (!except.contains(threads[i])) {
- if ((threads[i].getName().indexOf("VM") == -1) &&
- (threads[i].getName().indexOf("CompilerTh") == -1) &&
- (threads[i].getName().indexOf("Signal") == -1)) {
- System.out.println("suspending " + threads[i]);
- threads[i].suspend();
- }
- }
- }
- }
-
- @SuppressWarnings("deprecation")
- private static void resumeAllThreads() {
- Thread[] threads = getAllThreads();
-
- for (int i=0; i<threads.length; i++) {
- threads[i].resume();
- }
- }
-
private static class ClassInfo {
- private static Map<Class, ClassInfo> classInfoRegistry = new WeakHashMap<Class, ClassInfo>();
- private int size;
+ private static final Map<Class<?>, ClassInfo> classInfoRegistry = new WeakHashMap<>();
+ private final int size;
- private ClassInfo(Class cls) {
-// this.cls = cls;
+ private ClassInfo(Class<?> cls) {
if (cls.isArray()) {
- Class base = cls.getComponentType();
+ Class<?> base = cls.getComponentType();
size = -getSize(base, true);
} else {
// iterate all fields and sum the sizes
int sum = 0;
- for (Class act = cls; act != null; act = act.getSuperclass()) {
- Field[] flds = act.getDeclaredFields();
- for (int i=0; i<flds.length; i++) { // count all nonstatic fields
- if ((flds[i].getModifiers() & Modifier.STATIC) == 0) {
- sum += getSize(flds[i].getType(), false);
+ for (Class<?> act = cls; act != null; act = act.getSuperclass()) {
+ // count all nonstatic fields
Review Comment:
I copied the comment from original line 285 when I changed indext loop into for-each loop
if it's misleading then shall I remove 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] lbownik commented on pull request #5317: code cleanup - no logic changes
Posted by "lbownik (via GitHub)" <gi...@apache.org>.
lbownik commented on PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#issuecomment-1407417302
@vieiro - Yes. I will write tests. Meanwhile mark this PR as "do-not-merge" or something.
An one more thing.
The module does not mention any public packages in
https://github.com/apache/netbeans/blob/master/harness/o.n.insane/nbproject/project.xml
does it mean that it does not expose any APIs to other molecules or that every public class in all packages is exposed?
--
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] lbownik commented on pull request #5317: code cleanup - no logic changes
Posted by "lbownik (via GitHub)" <gi...@apache.org>.
lbownik commented on PR #5317:
URL: https://github.com/apache/netbeans/pull/5317#issuecomment-1407445628
@vieiro
> Good news! We'll wait for those tests them. Remember that we prefer _new_ tests, without modifying existing ones. The idea here is that new tests can replace existing ones later on, if needed. That will be easier to review for us.
Ok. I will add comments to existing tests specifying which new tests obsolete them.
> > The module does not mention any public packages in https://github.com/apache/netbeans/blob/master/harness/o.n.insane/nbproject/project.xml does it mean that it does not expose any APIs to other molecules or that every public class in all packages is exposed?
>
> Not all modules expose APIs to other modules, only if they have something interesting to expose to the world. This one exposes nothing.
This module's properties mentions "Export packages onlly to friends" and mentions 3 packages.
Does it mean that it is only used by these three mentioned packages?
If so does it make sense to test only API calls that are actually used by those packages and maybe later remove all the unused code (code not covered by new tests)?
--
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