Skip to content

Fix/adc dispatch perf overflow - #161

Open
jalinei wants to merge 2 commits into
owntech-foundation:mainfrom
jalinei:fix/adc-dispatch-perf-overflow
Open

jalinei wants to merge 2 commits into
owntech-foundation:mainfrom
jalinei:fix/adc-dispatch-perf-overflow

Conversation

@jalinei

@jalinei jalinei commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR improves ADC dispatch performance and fixes an out-of-bounds write when a channel buffer is full.

Performance improvement

Replace per-sample modulo operations with increment-and-wrap indexing, and update the task-mode DMA cursor once per dispatch. Sample order and existing __STATIC_INLINE helpers are preserved.

Buffer overflow fix

Previously, the sample count stopped increasing at CHANNELS_BUFFERS_SIZE, but the dispatcher continued writing at that index, one element beyond the buffer.

The dispatcher now skips writes when the channel buffer is full while continuing to advance through the DMA data. Buffered samples remain available until consumed.

Scope

Two commits, changing only data_dispatch.cpp. Existing formatting and helper functions are retained.

Validation

  • Passed 260 ASan/UBSan cases covering all five ADCs, task and interrupt modes, partial scans, ring wraps, buffer saturation, and buffer swaps.
  • Confirmed non-saturating sample output matches upstream.

Replace per-sample modulo with increment-and-wrap indexing and update the task DMA cursor once per dispatch. Preserve the existing inline buffer helpers and sample order.
Skip the write when a channel buffer is full while continuing to advance the DMA cursor. Retain buffered samples until consumption and preserve the inline count helper.
@jalinei
jalinei requested a review from cfoucher-laas October 14, 2026 13:27
@cfoucher-laas

cfoucher-laas commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Hello,

After some time understanding the details of the first diff, I see two main factors of improvement:

  • next_dma_buffer_index[adc_index] is not updated within the loop, only a temporary variable dma_buffer_index is. The array is only updated after the loop. This prevents computing the array indirection multiple times.
  • The expensive modulo operations on dma_buffer_index and channel_index are replaced by simple tests.

After careful checks, this diff should be functionally equivalent to the previous code.

Regarding the second diff, it is an obvious buffer overflow fix (we should move to rust to avoid these 😁).

However, I still see room for a very little improvement: channel_count could be assigned value at the beginning of the function, and the two remaining calls to enabled_channels_count[adc_index] replaced by this variable.
This could avoid 2 indirections if the compiler fails to cache it. Not sure it will add anything (depends if the compiler already did this optimization, plus it is not in the loop), but it is free to add it.

@cfoucher-laas cfoucher-laas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment in conversation.

}

static size_t next_dma_buffer_index[ADC_COUNT] = {0};
size_t channel_count = enabled_channels_count[adc_index];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move to the beginning of the function, and replace remaining calls to enabled_channels_count[adc_index] by channel_count.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants