Skip to content

Commit b988b37

Browse files
runningcodeclaude
andauthored
perf(core): Use fixed-delay scheduling for performance collector (JAVA-555) (#5524)
* perf(core): Use fixed-delay scheduling for performance collector Switch the transaction collection timer from scheduleAtFixedRate to schedule. Fixed-rate scheduling fires rapid catch-up executions after a delay or GC pause, which the old code guarded against with a 10ms skip check. Fixed-delay scheduling spaces each collection 100ms after the previous one finishes, so the catch-up bursts cannot happen and the guard, its timestamp field, and the stale comment are no longer needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * changelog * test(core): Verify schedule instead of scheduleAtFixedRate The performance collector now uses fixed-delay scheduling, so the timer verifications assert schedule(...) rather than scheduleAtFixedRate(...). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * changelog --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 8330a1b commit b988b37

3 files changed

Lines changed: 13 additions & 18 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66

77
- Reduce unboxing in `DateUtils.nanosToDate` ([#5523](https://github.com/getsentry/sentry-java/pull/5523))
88

9+
### Fixes
10+
11+
- Fix performance collector scheduling many tasks in a row ([#5524](https://github.com/getsentry/sentry-java/pull/5524))
12+
913
## 8.43.2
1014

1115
### Improvements

sentry/src/main/java/io/sentry/DefaultCompositePerformanceCollector.java

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ public final class DefaultCompositePerformanceCollector implements CompositePerf
2727

2828
private final @NotNull SentryOptions options;
2929
private final @NotNull AtomicBoolean isStarted = new AtomicBoolean(false);
30-
private long lastCollectionTimestamp = 0;
3130

3231
public DefaultCompositePerformanceCollector(final @NotNull SentryOptions options) {
3332
this.options = Objects.requireNonNull(options, "The options object is required.");
@@ -112,16 +111,8 @@ public void run() {
112111
new TimerTask() {
113112
@Override
114113
public void run() {
115-
long now = System.currentTimeMillis();
116-
// The timer is scheduled to run every 100ms on average. In case it takes longer,
117-
// subsequent tasks are executed more quickly. If two tasks are scheduled to run in
118-
// less than 10ms, the measurement that we collect is not meaningful, so we skip it
119-
if (now - lastCollectionTimestamp <= 10) {
120-
return;
121-
}
122114
timedOutTransactions.clear();
123115

124-
lastCollectionTimestamp = now;
125116
final @NotNull PerformanceCollectionData tempData =
126117
new PerformanceCollectionData(options.getDateProvider().now().nanoTimestamp());
127118

@@ -147,7 +138,7 @@ public void run() {
147138
}
148139
}
149140
};
150-
timer.scheduleAtFixedRate(
141+
timer.schedule(
151142
timerTask,
152143
TRANSACTION_COLLECTION_INTERVAL_MILLIS,
153144
TRANSACTION_COLLECTION_INTERVAL_MILLIS);

sentry/src/test/java/io/sentry/DefaultCompositePerformanceCollectorTest.kt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class DefaultCompositePerformanceCollectorTest {
8686
val collector = fixture.getSut(null, null)
8787
assertTrue(fixture.options.performanceCollectors.isEmpty())
8888
collector.start(fixture.transaction1)
89-
verify(fixture.mockTimer, never())!!.scheduleAtFixedRate(any(), any<Long>(), any())
89+
verify(fixture.mockTimer, never())!!.schedule(any(), any<Long>(), any())
9090
}
9191

9292
@Test
@@ -104,22 +104,22 @@ class DefaultCompositePerformanceCollectorTest {
104104
fun `when start, timer is scheduled every 100 milliseconds`() {
105105
val collector = fixture.getSut()
106106
collector.start(fixture.transaction1)
107-
verify(fixture.mockTimer)!!.scheduleAtFixedRate(any(), any<Long>(), eq(100))
107+
verify(fixture.mockTimer)!!.schedule(any(), any<Long>(), eq(100))
108108
}
109109

110110
@Test
111111
fun `when start with a string, timer is scheduled every 100 milliseconds`() {
112112
val collector = fixture.getSut()
113113
collector.start(fixture.id1)
114-
verify(fixture.mockTimer)!!.scheduleAtFixedRate(any(), any<Long>(), eq(100))
114+
verify(fixture.mockTimer)!!.schedule(any(), any<Long>(), eq(100))
115115
}
116116

117117
@Test
118118
fun `when stop, timer is stopped`() {
119119
val collector = fixture.getSut()
120120
collector.start(fixture.transaction1)
121121
collector.stop(fixture.transaction1)
122-
verify(fixture.mockTimer)!!.scheduleAtFixedRate(any(), any<Long>(), eq(100))
122+
verify(fixture.mockTimer)!!.schedule(any(), any<Long>(), eq(100))
123123
verify(fixture.mockTimer)!!.cancel()
124124
}
125125

@@ -128,15 +128,15 @@ class DefaultCompositePerformanceCollectorTest {
128128
val collector = fixture.getSut()
129129
collector.start(fixture.id1)
130130
collector.stop(fixture.id1)
131-
verify(fixture.mockTimer)!!.scheduleAtFixedRate(any(), any<Long>(), eq(100))
131+
verify(fixture.mockTimer)!!.schedule(any(), any<Long>(), eq(100))
132132
verify(fixture.mockTimer)!!.cancel()
133133
}
134134

135135
@Test
136136
fun `stopping a not collected transaction return null`() {
137137
val collector = fixture.getSut()
138138
val data = collector.stop(fixture.transaction1)
139-
verify(fixture.mockTimer, never())!!.scheduleAtFixedRate(any(), any<Long>(), eq(100))
139+
verify(fixture.mockTimer, never())!!.schedule(any(), any<Long>(), eq(100))
140140
verify(fixture.mockTimer, never())!!.cancel()
141141
assertNull(data)
142142
}
@@ -145,7 +145,7 @@ class DefaultCompositePerformanceCollectorTest {
145145
fun `stopping a not collected id return null`() {
146146
val collector = fixture.getSut()
147147
val data = collector.stop(fixture.id1)
148-
verify(fixture.mockTimer, never())!!.scheduleAtFixedRate(any(), any<Long>(), eq(100))
148+
verify(fixture.mockTimer, never())!!.schedule(any(), any<Long>(), eq(100))
149149
verify(fixture.mockTimer, never())!!.cancel()
150150
assertNull(data)
151151
}
@@ -316,7 +316,7 @@ class DefaultCompositePerformanceCollectorTest {
316316
collector.close()
317317

318318
// Timer was canceled
319-
verify(fixture.mockTimer)!!.scheduleAtFixedRate(any(), any<Long>(), eq(100))
319+
verify(fixture.mockTimer)!!.schedule(any(), any<Long>(), eq(100))
320320
verify(fixture.mockTimer)!!.cancel()
321321

322322
// Data was cleared

0 commit comments

Comments
 (0)