Skip to content

[AI Task] [Tizen.Sensor] Cache Marshal.SizeOf in Sensor batch event hot path#7739

Open
JoonghyunCho wants to merge 2 commits into
mainfrom
ai-task/issue-7658
Open

[AI Task] [Tizen.Sensor] Cache Marshal.SizeOf in Sensor batch event hot path#7739
JoonghyunCho wants to merge 2 commits into
mainfrom
ai-task/issue-7658

Conversation

@JoonghyunCho

Copy link
Copy Markdown
Member

Summary

Sensor.updateBatchEvents runs on the native sensor callback hot path and called Marshal.SizeOf<Interop.SensorEventStruct>() on every loop iteration while marshalling batched events. Batch sensors (e.g. HRM Batch, motion sensors) can deliver tens to hundreds of events per callback at 100Hz+, so the per-iteration reflection/marshalling metadata lookup accumulates measurable overhead.

Since the layout of Interop.SensorEventStruct is fixed at compile time, the size is now computed once into a static readonly field.

Changes

  • src/Tizen.Sensor/Tizen.Sensor/Sensor.cs
    • Added private static readonly int s_sensorEventStructSize = Marshal.SizeOf<Interop.SensorEventStruct>();
    • updateBatchEvents advances the event pointer by the cached size instead of calling Marshal.SizeOf<T>() per iteration (N calls → 0 per batch, where N = events_count).

Mode

Refactoring

Verification

  • Build: passed (dotnet build on Tizen.Sensor — 0 errors)
  • Tests: N/A
  • Benchmark: skipped (sdb error: no device attached — sdb devices lists no targets)

Fixes #7658

🤖 Generated with Claude Code

…7658)

Sensor.updateBatchEvents is invoked from native sensor callbacks and
called Marshal.SizeOf<Interop.SensorEventStruct>() on every loop
iteration. The struct layout is fixed at compile time, so the size is
now computed once into a static readonly field, removing per-event
reflection/marshalling metadata lookups from the batch hot path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@JoonghyunCho

Copy link
Copy Markdown
Member Author

⚠️ Benchmark skipped: Encountered an issue with sdb device connection/deployment/execution, so the benchmark could not be run. Manual benchmark verification is required. Error: sdb devices reports no attached devices (List of devices attached — empty).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request optimizes the Sensor class by caching the size of Interop.SensorEventStruct in a static readonly field, avoiding repeated calls to Marshal.SizeOf inside the updateBatchEvents loop. The reviewer suggested a further performance optimization to pre-allocate the capacity of the BatchedEvents list to prevent multiple internal array reallocations during the loop.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines 94 to 98
for (int i = 0; i < events_count; i++)
{
BatchedEvents.Add(Interop.IntPtrToEventStruct(currentPtr));
currentPtr += Marshal.SizeOf<Interop.SensorEventStruct>();
currentPtr += s_sensorEventStructSize;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Since updateBatchEvents is on a hot path and BatchedEvents is cleared and repopulated on every call, we can optimize memory allocations by ensuring the underlying List<T> has sufficient capacity before the loop. This avoids multiple internal array reallocations and copying overhead during the loop.

Since BatchedEvents is initialized as a List<Interop.SensorEventStruct>, we can cast it and set its Capacity if it is currently smaller than events_count.

                if (BatchedEvents is List<Interop.SensorEventStruct> list && list.Capacity < events_count)
                {
                    list.Capacity = (int)events_count;
                }

                for (int i = 0; i < events_count; i++)
                {
                    BatchedEvents.Add(Interop.IntPtrToEventStruct(currentPtr));
                    currentPtr += s_sensorEventStructSize;
                }

@JoonghyunCho

Copy link
Copy Markdown
Member Author

🤖 [AI Review]

Reviewed — no findings.

Scope checked:

  • Verified Marshal.SizeOf<Interop.SensorEventStruct>() is invariant for the process lifetime, so hoisting it into the new static readonly int s_sensorEventStructSize preserves the exact pointer-stride behavior of updateBatchEvents.
  • Confirmed the loop in updateBatchEvents was the only call site replaced, and IntPtr += int arithmetic is unchanged — BatchedEvents population order and count are identical.
  • The added field is private static — no public API surface change, no <since_tizen>/doc requirements apply.
  • Static initializer runs the same Marshal.SizeOf call previously executed per iteration, so no new failure mode is introduced at type load.

No 🔴 critical issues, no 🟡 suggestions to flag.


Automated review — final merge decision rests with human reviewers.

Pre-size BatchedEvents list capacity in updateBatchEvents to avoid
repeated internal array reallocations in the batch event hot path

Applied-Human-Comments: 3524742585
@JoonghyunCho

Copy link
Copy Markdown
Member Author

🤖 [AI Review]
Addressed review feedback in commit 7e2b6c0. Summary: pre-size the BatchedEvents list capacity in updateBatchEvents before the population loop, so the underlying List no longer performs repeated internal array reallocations on the batch event hot path.

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

Projects

None yet

1 participant