Community
Participate
Working Groups
Build ID: I20080617-2000 Steps To Reproduce: 1. Create an action that logs several IStatuses in a row in an attempt to get the log entries to have the same time stamp. Some kind of example code with a bundle activator: IStatus s1 = new Status(IStatus.INFO, Activator.PLUGIN_ID, "Test message 1", null); IStatus s2 = new Status(IStatus.WARNING, Activator.PLUGIN_ID, "Test message 2", null); IStatus s3 = new Status(IStatus.ERROR, Activator.PLUGIN_ID, "Test message 3", null); Activator.getDefault().getLog().log(s1); Activator.getDefault().getLog().log(s2); Activator.getDefault().getLog().log(s3); IStatus s4 = new Status(IStatus.ERROR, Activator.PLUGIN_ID, "Test message 4", new Exception()); Activator.getDefault().getLog().log(s4); 2. Run the action, open the error log and sort the log by the Date column. More information: Expected results: Entries in ascending order by date: Test Message 1 Test Message 2 Test Message 3 Test Message 4 Actual result: Entries in ascending order by date: Test Message 3 Test Message 2 Test Message 1 Test Message 4 The same thing happens in reverse with Descending sort order also. Clearly the date sorting is not stable with respect to the logging order, it is actually reversing the logging order of similarly timestamped log entries. The problem lies in the date column comparator of LogView.getViewerComparator(byte sortType): if (date1 == date2) { int result = elements.indexOf(e2) - elements.indexOf(e1); if (DATE_ORDER == DESCENDING) result *= DESCENDING; return result; } This means that if e1.date < e2.date, the method will actually return 1 and not -1 as specified. Fix by swapping e1 and e2 into: int result = elements.indexOf(e1) - elements.indexOf(e2);
LogView is under PDE/UI, and Jacek is its prophet!
uhh what a shame. Tukka thanks for the detailed report, and Ben for watching!
I could reproduce the issue and solution proposed by Tuukka fixes it indeed. Tuukka, would you like to create a patch? :)
Created attachment 113014 [details] Stabilizes date sorting. With pleasure :).
Thanks for the patch! This will get in for 3.5M3
done. > 20090922 Thanks guys!
Created attachment 113131 [details] mylyn/context/zip
Sorry to break the party, but I discovered today that when using Session or Plug-in grouping, the sorting of group subentries seems to be broken again in the same way as before. I didn't have time to look at exactly why this happens yet. Should I have reopened the bug or should somebody else verify these findings before that?
reopening to investigate when plug-in and session grouping is used. Jacek?
Created attachment 113225 [details] Fixes log entry sorting when using Session/Plug-in grouping Managed to fix the issue, attached a patch, but the solution is probably not the cleanest possible one.
Jacek, can you review the patch? It looks expensive from a quick glance :D Thanks for your patches Tuukka!
I agree, its rather expensive in cases where you have matching timestamps. On the other hand its really not any more expensive than the previous version since ArrayList.indexOf(Object), which the previous version was already using, does exactly the same as the private int indexOf(Object[], Object) in the patch does. The only thing I can currently think of that would optimize these indexOf-invocation away is to store the indices of each entry in the LogEntry itself. Since the actual logging order of events should never change, it should be perfectly valid to do this. This way the indexOf() invocations would be replaced with simple field accesses in each LogEntry, which should be fast and the whole children-array retrieval at the beginning of compare() would not be needed.
Created attachment 113326 [details] Revised version of group sorting patch Spotted another problem in the previous patch. In cases where MultiStatus is used for logging, a LogEntry will be the parent of other LogEntry instances. Thus, the children-array used for indexOf queries must always be looked up from the parent AbstractEntry if one exists. I tried to clean the solution up a bit.
(In reply to comment #13) My understanding is that comparator should sort items with different timestamps correctly. If they have the same timestamp, they should be left in the same order as in log file. If some children appear to be in different order than in file, then maybe error is somewhere else. Unfortunately I'm totally out of the world this week, so I'll give this bug more time next week. Anyway, Tuukka thank you for your work on this bug.
(In reply to comment #14) > > My understanding is that comparator should sort items with different timestamps > correctly. If they have the same timestamp, they should be left in the same > order as in log file. > If some children appear to be in different order than in file, then maybe error > is somewhere else. With my latest patch this is exactly what i see happening happens. The previous solutions did not fix all cases. The latest patch makes to sorting work on all tree levels, not just the first child level. > Unfortunately I'm totally out of the world this week, so I'll give this bug > more time next week. > Anyway, Tuukka thank you for your work on this bug. You're welcome. It's was in my best interest to fix this since I've actually cloned the whole LogView component for my own purposes as another view and that is how I originally ran into this snag.
after thorough testing, this looks good. If you have any issues, please coment Jacek. > 20090924 Thanks for your work Tuukka. I am going to open a bug to have tests for the logview... it's long overdue.
