Don't poll more than needed in BSP. #2971
Conversation
|
would love some benchmarking results along with these changes so we can compare apples to apples and not just guess at what might be better. :) |
|
Yup - it'll be a bit tricky since I don't think we have any overhead benchmarks where we'd measure e.g. CPU usage of a change. Might work on it but wanted to throw this out there first |
|
Along with any of these benchmarks of the BSP, we should also include metrics output so we can see how many spans we're dropping because the queue is full. I suspect this is a confounding factor that might be skewing results. |
| @@ -261,9 +272,13 @@ private void exportCurrentBatch() { | |||
| return; | |||
| } | |||
|
|
|||
| for (ReadableSpan span : batch) { | |||
| batchData.add(span.toSpanData()); | |||
There was a problem hiding this comment.
For good or ill, moving this down here will make the CPU usage a bit more spiky, since we'll only do this conversion all at once when the queue is full, or when the timer expires. The current implementation will spread out the conversion smoothly, since it happens as each span is added to the queue. Which way is better? I'm not sure, honestly, but it's something to keep in mind.
There was a problem hiding this comment.
Yeah there's some tradeoff - I guess for a thread with a defined "interval", I'd sort of expect it to wake, work, sleep. We have a bit of a pathological behavior right now where it's fairly conceivable for this thread to be sleeping and waking in a thrashing manner if a span is recorded every 101ms for example - sleeping/waking is very expensive and I suspect we didn't intend that to be possible.
| @@ -127,10 +127,12 @@ public CompletableResultCode forceFlush() { | |||
| private long nextExportTime; | |||
|
|
|||
| private final BlockingQueue<ReadableSpan> queue; | |||
There was a problem hiding this comment.
Using ArrayDequeue would be better when draining in batches similar to https://github.com/sbandadd/opentelemetry-java/pull/1/files. But I think the most important thing is to come up with a CPU benchmark and compare different versions.
|
Superceded by #2983 |
Just whipped this up to compare against #2969. @sbandadd sorry if this overlaps with any approach you've tried.