Enqueue less

[MatthewKhouzam]

What it does

This PR adds pre-filtering to event dispatch to speed up trace parsing by reducing queue fill. Only events that may be read by analyses are enqueued, improving performance.

The following state providers are updated to implement event filtering:

• ActiveTidStateProvider
• KernelStateProvider
• KernelMemoryStateProvider
• KernelCpuUsageStateProvider
• CallStackStateProvider
• UstMemoryStateProvider

How to test

1. Open a large trace file (particularly kernel traces)
2. Observe the parsing time and memory usage (Use tracing!)
3. Compare with previous versions - parsing should be faster with reduced queue buildup (diff flamegraph)
4. Verify that all analyses still produce correct results (no events are incorrectly filtered out)

Follow-ups

Add more analyses.

• As an author, I have thoroughly tested my changes and carefully followed the instructions in this template

Summary by CodeRabbit

• Performance
  • Improved event filtering and caching across providers to reduce unnecessary processing and speed up analyses.
• Refactor
  • Standardized a new pre-filter hook so providers share consistent event-selection behavior.
• Behavior
  • Call stack analysis now processes all incoming events by default; several memory and kernel analyses now only consider known, relevant events.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a fast-fail hook considerEvent(ITmfEvent) to AbstractTmfStateProvider (default true) and implements provider-specific overrides that pre-filter events by name or predicate so only selected events reach existing eventHandle logic.

Changes

Cohort / File(s)	Summary
Base Framework tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/statesystem/AbstractTmfStateProvider.java	Adds protected considerEvent(ITmfEvent) (default true) and integrates it into the event loop to skip events before eventHandle.
CPU Usage Analysis analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/analysis/os/linux/core/cpuusage/KernelCpuUsageStateProvider.java	Overrides considerEvent() to accept only sched_switch events (filters by event name).
Kernel Memory Analysis analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/analysis/os/linux/core/kernelmemoryusage/KernelMemoryStateProvider.java	Caches memory event names (fEventKmemPageAlloc, fEventKmemPageFree) and a fToConsider set; overrides considerEvent() to restrict processing to cached alloc/free events and uses cached names in handlers.
Kernel State & TID Providers analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/internal/analysis/os/linux/core/kernel/KernelStateProvider.java, analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/analysis/os/linux/core/tid/ActiveTidStateProvider.java	KernelStateProvider: adds fValidEventNames set, initializes it from fEventNames, and overrides considerEvent() to accept known event names or syscall entry/exit events. ActiveTidStateProvider: overrides considerEvent() to accept only sched_switch events (filters by fSchedSwitch).
Call Stack & UST Memory Providers analysis/org.eclipse.tracecompass.analysis.profiling.core/src/org/eclipse/tracecompass/analysis/profiling/core/callstack/CallStackStateProvider.java, lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/internal/lttng2/ust/core/analysis/memory/UstMemoryStateProvider.java	CallStackStateProvider: makes considerEvent() concrete (returns true) and removes per-event guard in eventHandle. UstMemoryStateProvider: overrides considerEvent() to accept only events present in its fEventNames map.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant EP as EventProcessor
participant SP as AbstractTmfStateProvider
participant P as SpecificStateProvider
participant SS as StateSystem
EP->>SP: fetch next event
SP->>P: considerEvent(event)
alt considerEvent == false
P-->>SP: reject -> EP skips event
else considerEvent == true
P->>P: eventHandle(event)
P->>SS: update state system
SS-->>P: ack
end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hop the trace with tidy paws,
I sniff the names and learn their laws,
sched_switch, alloc, free — I keep,
I skip the crumbs that make a heap,
Happy ears, the states sleep deep.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Enqueue less' is vague and generic, failing to clearly convey the specific nature of the changes despite the PR description detailing event pre-filtering implementation across multiple state providers.	Consider a more descriptive title such as 'Add event pre-filtering to reduce queue enqueuing during trace parsing' to better reflect the actual implementation.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In
`@analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/analysis/os/linux/core/tid/ActiveTidStateProvider.java`:
- Around line 75-78: The filter in considerEvent currently excludes sched_switch
events (it uses !event.getName().equals(fSchedSwitch)) which is inverted and
causes eventHandle (which reads fNextTid via layout.fieldNextTid()) to operate
on events without that field; change considerEvent to only accept sched_switch
events by removing the negation so it returns true when
event.getName().equals(fSchedSwitch), ensuring eventHandle sees events that
contain fNextTid; verify fSchedSwitch and fNextTid references (and
layout.fieldNextTid()) remain unchanged.

In
`@analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/internal/analysis/os/linux/core/kernel/KernelStateProvider.java`:
- Line 105: fValidEventNames is populated but never read (dead code); remove the
field declaration fValidEventNames and any code that populates it (references to
fValidEventNames), and then remove the now-unused Set/HashSet imports; ensure
considerEvent continues to use fEventNames.containsKey(name) so behavior is
unchanged.
- Around line 17-21: Remove the unused imports ArrayList and List from
KernelStateProvider.java to fix the CI build; locate the import lines
referencing java.util.ArrayList and java.util.List in the import block at the
top of the file and delete those two import statements so only the actually used
imports (e.g., HashSet, Map, Set) remain.

🧹 Nitpick comments (3)

tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/statesystem/AbstractTmfStateProvider.java (1)

443-455: Javadoc references "function entry/exit analysis" — too narrow for this general-purpose hook.

The Javadoc on line 444 says "function entry/exit analysis," which appears to be copied from CallStackStateProvider. Since this method lives in the abstract base class and is used by many different providers (kernel, memory, CPU usage, TID), the description should reflect its general filtering purpose.

📝 Suggested Javadoc fix

     /**
-     * Check if this event should be considered at all for function entry/exit
-     * analysis. This check is a fast fail to allow events to not be processed as much.
+     * Check if this event should be considered for state change processing.
+     * This is a fast-fail filter to skip irrelevant events early.
      *
      * `@param` event
      *            The event to check
      * `@return` If false, the event will be ignored by the state provider. If
      *         true processing will continue.
      * `@since` 10.2
      */

lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/internal/lttng2/ust/core/analysis/memory/UstMemoryStateProvider.java (1)

162-168: LGTM! Minor: missing space after if on line 164.

The filtering logic is correct and aligns well with the existing eventHandle logic.

-        if(!super.considerEvent(event)) {
+        if (!super.considerEvent(event)) {

analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/internal/analysis/os/linux/core/kernel/KernelStateProvider.java (1)

183-194: Minor formatting issues on lines 190 and 193.

• Line 190: trailing space before ): fEventNames.containsKey(name) )
• Line 193: missing space before ||: isSyscallEntry(name)|| isSyscallExit(name)

📝 Proposed fix

-        if (fEventNames.containsKey(name) ) {
+        if (fEventNames.containsKey(name)) {
             return true;
         }
-        return isSyscallEntry(name)|| isSyscallExit(name);
+        return isSyscallEntry(name) || isSyscallExit(name);

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In
`@analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/internal/analysis/os/linux/core/kernel/KernelStateProvider.java`:
- Around line 181-192: In considerEvent(ITmfEvent event) clean up the minor
formatting: remove the trailing space inside the contains call so
"fValidEventNames.contains(name) )" becomes "fValidEventNames.contains(name)"
and add a space before the logical operator so "isSyscallEntry(name)||
isSyscallExit(name)" becomes "isSyscallEntry(name) || isSyscallExit(name)"; keep
all logic unchanged and only adjust whitespace around the contains call and the
|| operator.

In
`@lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/internal/lttng2/ust/core/analysis/memory/UstMemoryStateProvider.java`:
- Around line 162-168: In UstMemoryStateProvider.override considerEvent, fix the
style issue by adding a space after the if keyword: change the condition check
in considerEvent (the line currently "if(!super.considerEvent(event))") to use
"if (!super.considerEvent(event))" so the method body remains the same (still
returns false and then checks fEventNames.containsKey(event.getName())).

In
`@tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/statesystem/AbstractTmfStateProvider.java`:
- Around line 443-455: The Javadoc for the protected method
considerEvent(ITmfEvent event) in AbstractTmfStateProvider incorrectly
references "function entry/exit analysis" which is too specific; update the
method comment to describe it as a general-purpose fast-fail hook for whether an
event should be processed by any state provider (e.g., kernel, memory,
CPU-usage, callstack), explaining that returning false skips processing and
returning true continues processing, and keep the `@since` and param/return tags
intact; locate the considerEvent method in AbstractTmfStateProvider and replace
the specific wording with a generic description that applies to all state
providers.

🧹 Nitpick comments (3)

analysis/org.eclipse.tracecompass.analysis.profiling.core/src/org/eclipse/tracecompass/analysis/profiling/core/callstack/CallStackStateProvider.java (1)

204-207: Redundant override—parent class already defaults to return true.

The parent AbstractTmfStateProvider defines considerEvent() with a default implementation that returns true, and the event filter is executed in EventProcessor.run() before eventHandle() is dispatched. This override repeats the parent's default behavior and adds no new functionality.

However, the override is defensible as documentation: it explicitly preserves the pre-existing Javadoc contract for CallStackStateProvider subclasses and signals that this provider does not perform event filtering. If the intent is to document this behavior for subclass authors, the override and its Javadoc are reasonable. If clarity is the only goal, consider removing it.

tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/statesystem/AbstractTmfStateProvider.java (1)

322-325: Pre-filter executes post-dequeue, not pre-enqueue — queue pressure is unchanged.

The commit message states the goal is to "reduce queue fill by enqueuing only events that may be read by analyses," but considerEvent is called after fEventsQueue.take() (consumer side), so every event still passes through the BufferedBlockingQueue. The optimization skips the heavier eventHandle path, which is valuable, but queue memory pressure and blocking in processEvent remain the same.

If the intent is truly to reduce enqueue volume, consider moving the considerEvent check into processEvent() (producer side):

♻️ Sketch: filter on the producer side

 `@Override`
 public void processEvent(ITmfEvent event) {
     if (!fStateSystemAssigned) {
         throw new IllegalStateException(...);
     }
     fPropagateExceptions.run();
-    ITmfEvent curEvent = event;
-    fEventsQueue.put(curEvent);
+    if (considerEvent(event)) {
+        fEventsQueue.put(event);
+    }
 }

Note: moving the check to the producer side would change the threading model — considerEvent would then run on the producer thread instead of the consumer thread. If subclass overrides access fields that are only safe on the consumer thread, this could introduce concurrency issues. Evaluate whether the subclass implementations are thread-safe before making this change.

analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/analysis/os/linux/core/kernelmemoryusage/KernelMemoryStateProvider.java (1)

64-68: TreeSet is unnecessary for a 2-element membership-check set.

fToConsider is only ever used for contains() calls with exactly two elements. A HashSet (like KernelStateProvider uses) would be more idiomatic for unordered membership tests. TreeSet adds ordering overhead that isn't leveraged here.

♻️ Suggested change

-import java.util.TreeSet;
+import java.util.HashSet;

-    private final Set<String> fToConsider = new TreeSet<>();
+    private final Set<String> fToConsider = new HashSet<>();