Community
Participate
Working Groups
Created attachment 200664 [details] Proposed patch to combine Thread Overview and Thread Stacks queries Eliminate some confusion by combining Thread Overview and Thread Stacks queries. The Thread Stacks query essentially becomes the basis with blank column values for the extra columns brough over from Overview on child rows. The context menu for Thread Details for the thread row also comes over from Overview to Stacks which is beneficial. Keep Thread Details query because it may contain extra information such as native stack.
Kevin, thanks for sending us ideas and patches! I tried this patch today, as well as the patches from bug 353341 and bug 352230, which will eventually become obsolete if the patch from this message is applied. I think the changes are reasonable, however there are still some problems with the patch in this ticket: 1) For some threads there are no stacks, therefore a NullPointerException was coming in some cases. I could fix this by midifying your patch like this: public boolean hasChildren(Object element) { if (element instanceof ThreadOverviewNode) { /* added this here to fix the NPE */ IThreadStack stack = ((ThreadOverviewNode) element).stack; if (stack == null) return false; /* END added this here to fix the NPE */ IStackFrame[] frames = stack.getStackFrames(); return frames != null && frames.length > 0; } . . . 2) Sorting on the "Class Name" column also sorts the stack frames, which should be avoided 3) If I run the query on a selection of objects which are not threads, then get some results with strange content of the columns. Non thread objects should be filtered (this applies also for bug 353341) How shall we proceed? I could apply the smaller changes from the other two bugs first (fixing 3) as mentioned). Then you'll need to revise the patch attached here. Alternatively, we could try to resolve the problems at once with the patch from this ticket. This introduces more changes at once, so I'll probably need more time to play with it. What do you think?
Created attachment 200815 [details] Proposed patch to combine Thread Overview and Thread Stacks queries V2
> Kevin, thanks for sending us ideas and patches! > I tried this patch today, as well as the patches from bug 353341 and bug > 352230, which will eventually become obsolete if the patch from this message is Hi Krum, thanks very much for taking a look. I've attached a new patch, attachment 200815 [details], which will hopefully resolve these bugs. I have some comments below: > /* added this here to fix the NPE */ > IThreadStack stack = ((ThreadOverviewNode) element).stack; > if (stack == null) > return false; Accepted > 2) Sorting on the "Class Name" column also sorts the stack frames, which should > be avoided Added the NoCompareComparator back from ThreadStacksQuery > > 3) If I run the query on a selection of objects which are not threads, then get > some results with strange content of the columns. Non thread objects should be > filtered (this applies also for bug 353341) Is there a helper method to check if an IObject extends some class or do I have to walk the class hierarchy? > > How shall we proceed? I'd like to pursue this bug even if it takes longer and we can close bug 353341 and bug 352230
Created attachment 200823 [details] Proposed patch to combine Thread Overview and Thread Stacks queries V3
> Is there a helper method to check if an IObject extends some class or do I have > to walk the class hierarchy? Uploaded another patch, attachment 200823 [details], with the following check: public static final String CLASS_THREAD = "java.lang.Thread"; ... public static boolean isThread(ISnapshot snapshot, int objectId) throws SnapshotException { IObject obj = snapshot.getObject(objectId); IClass cls = obj.getClazz(); if (cls != null) { String className = cls.getName(); if (CLASS_THREAD.equals(className)) { return true; } while (cls.hasSuperClass()) { cls = cls.getSuperClass(); className = cls.getName(); if (CLASS_THREAD.equals(className)) { return true; } } } return false; }
I don't think we have a method for checking this. This could be a reasonable extension of the API. An alternative way to check this is to get once all the classes and subclasses of "java.lang.Thread", i.e. snapshot.getClassesByName("java.lang.Thread", true) then remember all classIds in a Set. Afterwards just check if the classId of the object is in the set. There is a org.eclipse.mat.collect.SetInt class for storing primitive ints. About the patch - sorry for not replying earlier. I think the patch is now fine and I'd like to submit it. I just wonder in which version. We recently released the 1.1.0 and since then there were a few bugfixes. I'd like to release a 1.1.1 just with the bug-fixes. The current change I see more as a functional change, and would like to add it to the next bigger release - be it 1.2 or 2.0. More concretely this means - I'll make a 1.1.1 in the next 1-2 weeks and will then branch 1.1.x . After this I will submit also your change into trunc and it will be available with the next milestone build. Is this OK with you?
(In reply to comment #6) > After this > I will submit also your change into trunc and it will be available with the > next milestone build. Is this OK with you? Hi Krum, that's fine with me. ~Kevin
Fine. Then I'll update the ticket when there is some progress.
Comment on attachment 200823 [details] Proposed patch to combine Thread Overview and Thread Stacks queries V3 I've submitted the proposed patch (revision 1181 in SVN).
Kevin, thanks again for your contribution! I am closing the ticket now. If you have any further questions/comments you can reopen it.
Thanks very much Krum!
There is a difference in columns between the new combined overview and stacks and the separate overview and stacks. The new query doesn't show the thread name separately, which is actually quite an important attribute and it is useful to sort by this view too. Is it worth adding a thread name column, not putting the thread name into the the object column and perhaps renaming the object column 'Object / Stack Frame'? Which should be the first column? There's another problem - trees cannot be added to the compare basket, but tables can. This means we cannot now compare thread overview/stacks. Is this a problem? How hard would it be to either allow trees to be compared (only unexpanded?) or generate a table from a tree. Thead overview and stacks: Class Name org.eclipse.equinox.internal.util.impl.tpt.threadpool.Executor @ 0x7ffe4e172b8 null |- at java.lang.Object.wait(JI)V (Native Method) | '- <local> org.eclipse.equinox.internal.util.impl.tpt.threadpool.Executor @ 0x7ffe4e172b8 Thread Thread overview: Name |Instance Scheduled Executor-thread-1|java.lang.Thread @ 0xd6f03dc8 Events-thread-0 |com.ibm.ws.threading.internal.ThreadImpl @ 0xd73370f0 Thread stacks: Object / Stack Frame java.lang.Thread @ 0xd6f03dc8 Scheduled Executor-thread-1 |- at sun.misc.Unsafe.park(ZJ)V (Native Method) | '- <local> sun.misc.Unsafe @ 0xd69f8c40
*** Bug 353341 has been marked as a duplicate of this bug. ***
*** Bug 352230 has been marked as a duplicate of this bug. ***
(In reply to comment #12) > There is a difference in columns between the new combined overview and stacks > and the separate overview and stacks. > The new query doesn't show the thread name separately, which is actually quite > an important attribute and it is useful to > sort by this view too. Is it worth adding a thread name column, not putting the > thread name into the the object column and perhaps renaming the object column > 'Object / Stack Frame'? Which should be the first column? Hey Andrew, I'm running revision 1189 with this patch and I can see a separate, sortable name column (defined by ThreadInfoImpl_Column_Name). > There's another > problem - trees cannot be added to the compare basket, but tables can. This > means we cannot now compare thread overview/stacks. Is this a problem? How hard > would it be to either allow trees to be compared (only unexpanded?) or generate > a table from a tree. Also, I was able to add the thread_overview query to the compare basket.
I've added the extra columns using bug 360925 and allowed comparison of trees with bug 361631.
(In reply to comment #16) > I've added the extra columns using bug 360925 and allowed comparison of trees > with bug 361631. Ahh, well done, thanks Andrew!
