Skip to content

Commit 6028691

Browse files
watsonwatson
authored
refactor(debugger): optimize JSONBuffer timer management (#7365)
1 parent 7b348ca commit 6028691

File tree

2 files changed

+240
-41
lines changed

2 files changed

+240
-41
lines changed

‎packages/dd-trace/src/debugger/devtools_client/json-buffer.js‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,26 +11,25 @@ class JSONBuffer {
1111
this.#maxSize = size
1212
this.#timeout = timeout
1313
this.#onFlush = onFlush
14-
this.#reset()
15-
}
16-
17-
#reset () {
18-
clearTimeout(this.#timer)
19-
this.#timer = undefined
20-
this.#partialJson = undefined
2114
}
2215

2316
#flush () {
2417
const json = `${this.#partialJson}]`
25-
this.#reset()
18+
this.#partialJson = undefined
2619
this.#onFlush(json)
2720
}
2821

2922
write (str, size = Buffer.byteLength(str)) {
30-
if (this.#timer === undefined) {
23+
if (this.#partialJson === undefined) {
3124
this.#partialJson = `[${str}`
32-
this.#timer = setTimeout(() => this.#flush(), this.#timeout)
33-
} else if (Buffer.byteLength(/** @type {string} */ (this.#partialJson)) + size + 2 > this.#maxSize) {
25+
if (this.#timer === undefined) {
26+
this.#timer = setTimeout(() => this.#flush(), this.#timeout)
27+
} else {
28+
this.#timer.refresh()
29+
}
30+
} else if (Buffer.byteLength(this.#partialJson) + size + 2 > this.#maxSize) {
31+
clearTimeout(this.#timer)
32+
this.#timer = undefined
3433
this.#flush()
3534
this.write(str, size)
3635
} else {

‎packages/dd-trace/test/debugger/devtools_client/json-buffer.spec.js‎

Lines changed: 230 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,248 @@
11
'use strict'
22

33
const assert = require('node:assert/strict')
4+
const sinon = require('sinon')
45

5-
const { describe, it } = require('mocha')
6+
const { describe, it, beforeEach, afterEach } = require('mocha')
67
require('../../setup/mocha')
78

89
const JSONBuffer = require('../../../src/debugger/devtools_client/json-buffer')
910

1011
const MAX_SAFE_SIGNED_INTEGER = 2 ** 31 - 1
1112

1213
describe('JSONBuffer', () => {
13-
it('should call onFlush with the expected payload when the timeout is reached', function (done) {
14-
const onFlush = (json) => {
15-
const diff = Date.now() - start
16-
assert.strictEqual(json, '[{"message":1},{"message":2},{"message":3}]')
17-
assert.ok(((diff) >= (95) && (diff) <= (110)))
18-
done()
19-
}
20-
21-
const jsonBuffer = new JSONBuffer({ size: Infinity, timeout: 100, onFlush })
22-
23-
const start = Date.now()
24-
jsonBuffer.write(JSON.stringify({ message: 1 }))
25-
jsonBuffer.write(JSON.stringify({ message: 2 }))
26-
jsonBuffer.write(JSON.stringify({ message: 3 }))
14+
let clock
15+
16+
beforeEach(() => {
17+
clock = sinon.useFakeTimers()
18+
})
19+
20+
afterEach(() => {
21+
clock.restore()
2722
})
2823

29-
it('should call onFlush with the expected payload when the size is reached', function (done) {
30-
const expectedPayloads = [
31-
'[{"message":1},{"message":2}]',
32-
'[{"message":3},{"message":4}]'
33-
]
24+
describe('timeout-based flushing', () => {
25+
it('should call onFlush with the expected payload when the timeout is reached', function () {
26+
let flushedJson
27+
const onFlush = (json) => {
28+
flushedJson = json
29+
}
30+
31+
const jsonBuffer = new JSONBuffer({ size: Infinity, timeout: 100, onFlush })
32+
33+
jsonBuffer.write(JSON.stringify({ message: 1 }))
34+
jsonBuffer.write(JSON.stringify({ message: 2 }))
35+
jsonBuffer.write(JSON.stringify({ message: 3 }))
36+
37+
clock.tick(99)
38+
assert.strictEqual(flushedJson, undefined, 'Should not flush before timeout')
39+
40+
clock.tick(1)
41+
assert.strictEqual(flushedJson, '[{"message":1},{"message":2},{"message":3}]')
42+
})
43+
44+
it('should reset timeout when writing after a timeout-based flush', function () {
45+
let flushCount = 0
46+
const onFlush = () => { flushCount++ }
47+
48+
const jsonBuffer = new JSONBuffer({ size: Infinity, timeout: 100, onFlush })
49+
50+
jsonBuffer.write(JSON.stringify({ message: 1 }))
51+
52+
// Let first message flush via timeout
53+
clock.tick(100)
54+
assert.strictEqual(flushCount, 1, 'Should have flushed first message')
55+
56+
// Write again - timeout should be reset from this point
57+
jsonBuffer.write(JSON.stringify({ message: 2 }))
58+
59+
// Should need full 100ms from the second write
60+
clock.tick(99)
61+
assert.strictEqual(flushCount, 1, 'Should not flush yet')
62+
63+
clock.tick(1)
64+
assert.strictEqual(flushCount, 2, 'Should flush after full timeout period')
65+
})
66+
67+
it('should reset state properly after flush', function () {
68+
const flushedPayloads = []
69+
const onFlush = (json) => flushedPayloads.push(json)
70+
71+
const jsonBuffer = new JSONBuffer({ size: Infinity, timeout: 100, onFlush })
72+
73+
jsonBuffer.write(JSON.stringify({ message: 1 }))
74+
jsonBuffer.write(JSON.stringify({ message: 2 }))
75+
76+
clock.tick(100)
77+
assert.strictEqual(flushedPayloads.length, 1)
78+
assert.strictEqual(flushedPayloads[0], '[{"message":1},{"message":2}]')
79+
80+
// Write after flush - should start a new buffer
81+
jsonBuffer.write(JSON.stringify({ message: 3 }))
82+
jsonBuffer.write(JSON.stringify({ message: 4 }))
83+
84+
clock.tick(100)
85+
assert.strictEqual(flushedPayloads.length, 2)
86+
assert.strictEqual(flushedPayloads[1], '[{"message":3},{"message":4}]')
87+
})
88+
})
89+
90+
describe('size-based flushing', () => {
91+
it('should call onFlush with the expected payload when the size is reached', function () {
92+
const expectedPayloads = [
93+
'[{"message":1},{"message":2}]',
94+
'[{"message":3},{"message":4}]'
95+
]
96+
97+
const onFlush = (json) => {
98+
assert.strictEqual(json, expectedPayloads.shift())
99+
}
100+
101+
const jsonBuffer = new JSONBuffer({ size: 30, timeout: MAX_SAFE_SIGNED_INTEGER, onFlush })
102+
103+
jsonBuffer.write(JSON.stringify({ message: 1 })) // size: 15
104+
jsonBuffer.write(JSON.stringify({ message: 2 })) // size: 29
105+
jsonBuffer.write(JSON.stringify({ message: 3 })) // size: 15 (flushed, and re-added)
106+
jsonBuffer.write(JSON.stringify({ message: 4 })) // size: 29
107+
jsonBuffer.write(JSON.stringify({ message: 5 })) // size: 15 (flushed, and re-added)
108+
109+
assert.strictEqual(expectedPayloads.length, 0, 'All expected payloads should have been flushed')
110+
})
111+
112+
it('should handle writing exactly at the size limit', function () {
113+
let flushedJson
114+
const onFlush = (json) => {
115+
flushedJson = json
116+
}
117+
118+
// '[{"a":1},{"b":2}]' = 19 bytes exactly
119+
const jsonBuffer = new JSONBuffer({ size: 19, timeout: MAX_SAFE_SIGNED_INTEGER, onFlush })
120+
121+
jsonBuffer.write(JSON.stringify({ a: 1 })) // '[{"a":1}' = 8 bytes
122+
assert.strictEqual(flushedJson, undefined, 'Should not flush after first write')
123+
124+
jsonBuffer.write(JSON.stringify({ b: 2 })) // ',{"b":2}]' = 11 more bytes = 19 total
125+
assert.strictEqual(flushedJson, undefined, 'Should not flush when exactly at limit')
126+
127+
jsonBuffer.write(JSON.stringify({ c: 3 })) // Would exceed, so flush first
128+
assert.strictEqual(flushedJson, '[{"a":1},{"b":2}]')
129+
})
130+
131+
it('should handle a single message larger than maxSize', function () {
132+
const flushedPayloads = []
133+
const onFlush = (json) => flushedPayloads.push(json)
134+
135+
const jsonBuffer = new JSONBuffer({ size: 10, timeout: MAX_SAFE_SIGNED_INTEGER, onFlush })
136+
137+
const largeMessage = JSON.stringify({ message: 'very long message that exceeds size' })
138+
jsonBuffer.write(largeMessage) // Single message > 10 bytes
139+
140+
// Should still buffer it (no flush yet)
141+
assert.strictEqual(flushedPayloads.length, 0)
142+
143+
// Writing another message should flush the large one
144+
jsonBuffer.write(JSON.stringify({ msg: 2 }))
145+
assert.strictEqual(flushedPayloads.length, 1)
146+
assert.strictEqual(flushedPayloads[0], `[${largeMessage}]`)
147+
})
148+
149+
it('should use provided size parameter instead of calculating', function () {
150+
const flushedPayloads = []
151+
const onFlush = (json) => flushedPayloads.push(json)
152+
153+
const jsonBuffer = new JSONBuffer({ size: 20, timeout: MAX_SAFE_SIGNED_INTEGER, onFlush })
154+
155+
const msg1 = JSON.stringify({ msg: 1 })
156+
jsonBuffer.write(msg1) // 10 bytes
157+
assert.strictEqual(flushedPayloads.length, 0)
158+
159+
const msg2 = JSON.stringify({ msg: 2 })
160+
// Actual size would be 10 + 10 + 2 = 22, but claim it's only 5 bytes
161+
// This means 10 + 5 + 2 = 17 <= 20, so it won't flush
162+
jsonBuffer.write(msg2, 5)
163+
assert.strictEqual(flushedPayloads.length, 0, 'Should not flush due to fake size')
164+
165+
const msg3 = JSON.stringify({ msg: 3 })
166+
// Now actual partialJson is '[{"msg":1},{"msg":2}' = 21 bytes
167+
// Adding msg3: 21 + 10 + 2 = 33 > 20, so flush
168+
jsonBuffer.write(msg3)
169+
assert.strictEqual(flushedPayloads.length, 1)
170+
assert.strictEqual(flushedPayloads[0], `[${msg1},${msg2}]`)
171+
})
172+
173+
it('should clear timer when size limit is reached and create new timer on recursive write', function () {
174+
let flushCount = 0
175+
const onFlush = () => { flushCount++ }
176+
177+
const jsonBuffer = new JSONBuffer({ size: 30, timeout: 100, onFlush })
178+
179+
jsonBuffer.write(JSON.stringify({ message: 1 })) // 15 bytes, starts timer
180+
clock.tick(50) // Advance time but don't trigger timeout
181+
182+
jsonBuffer.write(JSON.stringify({ message: 2 })) // 29 bytes
183+
jsonBuffer.write(JSON.stringify({ message: 3 })) // Exceeds limit, flushes and recursively writes msg 3
184+
185+
assert.strictEqual(flushCount, 1, 'Should have flushed due to size')
186+
187+
// Original timer was cleared, so ticking 99ms (149ms total) shouldn't flush
188+
clock.tick(99)
189+
assert.strictEqual(flushCount, 1, 'Original timer was cleared, should not flush')
190+
191+
// New timer for message 3 needs full 100ms from the recursive write
192+
clock.tick(1)
193+
assert.strictEqual(flushCount, 2, 'Should flush message 3 after its full timeout')
194+
})
195+
})
196+
197+
describe('edge cases', () => {
198+
it('should handle rapid successive writes', function () {
199+
let flushedJson
200+
const onFlush = (json) => {
201+
flushedJson = json
202+
}
203+
204+
const jsonBuffer = new JSONBuffer({ size: Infinity, timeout: 100, onFlush })
205+
206+
for (let i = 1; i <= 10; i++) {
207+
jsonBuffer.write(JSON.stringify({ num: i }))
208+
}
209+
210+
clock.tick(100)
211+
assert.ok(flushedJson, 'Should have flushed data')
212+
const parsed = JSON.parse(flushedJson)
213+
assert.deepStrictEqual(parsed, [
214+
{ num: 1 }, { num: 2 }, { num: 3 }, { num: 4 }, { num: 5 },
215+
{ num: 6 }, { num: 7 }, { num: 8 }, { num: 9 }, { num: 10 }
216+
])
217+
})
218+
219+
it('should handle empty buffer timeout', function () {
220+
let flushCount = 0
221+
const onFlush = () => { flushCount++ }
222+
223+
// Create buffer but don't write anything - we're testing that nothing happens
224+
// eslint-disable-next-line no-new
225+
new JSONBuffer({ size: Infinity, timeout: 100, onFlush })
226+
227+
clock.tick(100)
228+
assert.strictEqual(flushCount, 0, 'Should not flush if nothing was written')
229+
})
230+
231+
it('should handle consecutive writes that each exceed size limit', function () {
232+
const flushedPayloads = []
233+
const onFlush = (json) => flushedPayloads.push(json)
34234

35-
const onFlush = (json) => {
36-
assert.strictEqual(json, expectedPayloads.shift())
37-
if (expectedPayloads.length === 0) done()
38-
}
235+
const jsonBuffer = new JSONBuffer({ size: 15, timeout: MAX_SAFE_SIGNED_INTEGER, onFlush })
39236

40-
const jsonBuffer = new JSONBuffer({ size: 30, timeout: MAX_SAFE_SIGNED_INTEGER, onFlush })
237+
jsonBuffer.write(JSON.stringify({ a: 1 })) // 8 bytes, buffered
238+
jsonBuffer.write(JSON.stringify({ b: 2 })) // 8 + 10 + 2 = 20 > 15, flush a, buffer b
239+
jsonBuffer.write(JSON.stringify({ c: 3 })) // 8 + 10 + 2 = 20 > 15, flush b, buffer c
240+
jsonBuffer.write(JSON.stringify({ d: 4 })) // 8 + 10 + 2 = 20 > 15, flush c, buffer d
41241

42-
jsonBuffer.write(JSON.stringify({ message: 1 })) // size: 15
43-
jsonBuffer.write(JSON.stringify({ message: 2 })) // size: 29
44-
jsonBuffer.write(JSON.stringify({ message: 3 })) // size: 15 (flushed, and re-added)
45-
jsonBuffer.write(JSON.stringify({ message: 4 })) // size: 29
46-
jsonBuffer.write(JSON.stringify({ message: 5 })) // size: 15 (flushed, and re-added)
242+
assert.strictEqual(flushedPayloads.length, 3)
243+
assert.strictEqual(flushedPayloads[0], '[{"a":1}]')
244+
assert.strictEqual(flushedPayloads[1], '[{"b":2}]')
245+
assert.strictEqual(flushedPayloads[2], '[{"c":3}]')
246+
})
47247
})
48248
})

0 commit comments

Comments
 (0)