Sum-detail traces: reconcile .sum idle with .sumd entry times, and speed up bar drawing - #159
Merged
Conversation
For sum-detail traces the per-interval entry method times come from the .sumd files while idle comes from the .sum files. charm's trace-summary accumulates the two through independent paths (SumLogPool::add() and binIdle vs updateSummaryDetail()) and they do not agree, so deriving overhead as 100 - work - idle produced negative values. On a 1920 PE, 32671 interval trace that tripped Time Profile's bad-data filter for 947 intervals, zeroing them and printing 1484 "log file corruption" warnings. Entry method times are what these charts are about, so treat them as authoritative and give idle only the time they leave free, rather than discarding the interval. Also fixes two ways the idle series was lined up wrongly against them: - getTotalIdlePercentagePerInterval() is indexed from interval 0 of the whole run, but graphData[0] is the first interval of the selected range. Any range not starting at 0 read idle from the wrong point in time. Now mapped through absolute time, and via getSummaryIntervalSize() so it holds if the .sum and .sumd interval sizes ever differ. - Entry method percentages are divided by the number of *selected* PEs while idle was averaged over *all* PEs, so a PE subset put the two on different scales. Adds a getTotalIdlePercentagePerInterval(SortedSet) overload that averages over the same PEs. Extrema's sum-detail branch had the same unguarded subtraction, plus a loop over every PE in the run writing into a tempData sized for the selected PEs only -- an ArrayIndexOutOfBoundsException whenever a subset was selected. It now iterates selectedPEs, which also matches the order peNames is built in. Usage Profile gets the same cap so a bar cannot exceed 100%. The residual disagreement is genuine rounding, well under the existing 5% tolerance (0.4-0.7% worst case on the trace above). Intervals that still fail the filter are reported as one summary line instead of one per interval. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
drawBarGraph issued one fillRect per (interval, entry method) pair. When more intervals are displayed than there are pixels, most bars come out zero pixels wide, and in a stacked chart most slices round to zero pixels tall; both draw nothing. A 31000 interval by 412 EP Time Profile spent minutes on the event dispatch thread pushing ~12.8M such no-op fills to the render queue. Work out each bar's horizontal extent once and skip the interval before touching its values when it is zero pixels wide, and skip slices that round to zero pixels tall. The unstacked branch now skips before its O(numY^2) selection sort as well. Output is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Time Profile shows a progress bar while reading .log files but had none for sum-detail traces, which drop into a silent serial loop expanding the RLE data. That loop costs numPEs * numIntervals * numEPs -- over nine billion iterations on a 1920 PE trace -- and looked like a hang. Drive a ProgressMonitor per PE, the same way SumAnalyzer does when reading .sum files, and honour cancellation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lvkale
requested review from
ericjbohm,
matthiasdiener and
ritvikrao
as code owners
July 29, 2026 18:58
ritvikrao
approved these changes
Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-on to #158, found while looking at a 1920 PE sum-detail trace (32671 intervals, 412 EPs). Three independent problems, one commit each.
Idle vs entry method times
Time Profile stacks per-interval entry method times from the
.sumdfiles against idle from the.sumfiles. charm'strace-summaryaccumulates those through independent paths (SumLogPool::add()/binIdlevsupdateSummaryDetail()) and they do not agree, sooverhead = 100 - work - idlecame out negative. That tripped the bad-data filter for 947 intervals, zeroing them and printing 1484 "log file corruption" warnings.Entry method times are what these charts are for, so they are now treated as authoritative and idle gets only the time they leave free. Two ways the idle series was also lined up wrongly against them are fixed:
getTotalIdlePercentagePerInterval()is indexed from interval 0 of the whole run, butgraphData[0]is the first interval of the selected range. Any range not starting at 0 read idle from the wrong point in time.Extrema's sum-detail branch had the same unguarded subtraction, plus a loop over every PE in the run writing into a
tempDatasized for the selected PEs only — anArrayIndexOutOfBoundsExceptionwhenever a subset was selected, so that path only ever worked with every PE selected.The residual disagreement is genuine rounding, 0.4–0.7% worst case on the trace above, well inside the existing 5% tolerance. Intervals that still fail the filter are now reported as one summary line rather than one per interval.
The underlying inconsistency is a charm-side bug, filed separately as charmplusplus/charm#3934.
Bar drawing
drawBarGraphissued onefillRectper (interval, entry method) pair. With more intervals than pixels most bars are zero pixels wide, and in a stacked chart most slices round to zero pixels tall; both draw nothing. A 31000 x 412 Time Profile spent minutes on the EDT pushing ~12.8M no-op fills. Now skipped, output unchanged.Progress bar
The sum-detail load had no progress bar — only the
.logpath did — and its serial RLE expansion costsnumPEs * numIntervals * numEPs, over nine billion iterations here. It looked like a hang.Testing
Built and smoke tested (
make test). Verified in the GUI on the 1920 PE trace above: Time Profile over the full range and over a sub-range with every 5th PE selected, Usage Profile, and Extrema.