Skip to content

perf: Reduce allocations across Shimmer, Popup, TabView, and TextInputLayout#389

Open
PaulAndersonS wants to merge 1 commit into
mainfrom
paulandersons/perf-shimmer-popup-tabview-textinput-opt
Open

perf: Reduce allocations across Shimmer, Popup, TabView, and TextInputLayout#389
PaulAndersonS wants to merge 1 commit into
mainfrom
paulandersons/perf-shimmer-popup-tabview-textinput-opt

Conversation

@PaulAndersonS

Copy link
Copy Markdown
Collaborator

Root Cause of the Issue

Multiple controls have unnecessary object allocations, redundant method calls, and expensive reflection operations in hot paths that negatively impact runtime performance.

Description of Change

This PR implements 5 targeted performance improvements across 4 controls:

  1. Shimmer – Cache Point allocations as static readonly fields

    • CreateWavePaint() previously created 2 new Point objects on every call. Since the values are constants (e.g., (0,0), (1,0), (0,1), (1,1)), they are now cached as static readonly fields, eliminating per-call heap allocations.
  2. Shimmer – Remove Cast<View>() LINQ allocation in DrawCustomViewChildren

    • The recursive DrawCustomViewChildren method called .Cast<View>() on layout.Children, which allocates an enumerator wrapper on every call. Replaced with direct iteration and type checking.
  3. Popup – Eliminate redundant null check and unnecessary new Page() allocations

    • GetMainPage() had a duplicate if (windowPage is not null) check (dead code).
    • Both GetMainPage() and GetMainWindowPage() returned new Page() as a fallback, creating garbage that callers immediately discarded. Now returns null, with callers updated for null-safety.
  4. TabView (iOS) – Cache GetProperties().Any() reflection result per-type

    • On every touch event, the handler called touchViewType.GetProperties().Any(...) which uses reflection to scan all properties. Added a ConcurrentDictionary<Type, bool> cache so reflection happens only once per type.
  5. TextInputLayout – Consolidate multiple InvalidateDrawable() calls

    • OnTextInputViewTextChanged could call InvalidateDrawable() up to 2 times in a single invocation (once for hint state change + once for clear icon). Consolidated into a single deferred call using a needsRedraw flag.

Issues Fixed

N/A – Proactive performance improvement.

Unit Tests

Added 11 unit tests in PerformanceOptimizationTests.cs covering:

  • Shimmer gradient point correctness for all wave directions
  • Shimmer cached static point verification
  • Popup null return behavior (no more unnecessary Page allocations)
  • TextInputLayout text change handler functionality
  • TabView type accessibility verification

…tLayout

- Shimmer: Cache Point allocations as static readonly fields in CreateWavePaint
- Shimmer: Remove LINQ Cast<View>() allocation in DrawCustomViewChildren
- Popup: Eliminate redundant null check and unnecessary new Page() allocations
- TabView (iOS): Cache reflection GetProperties().Any() result per-type
- TextInputLayout: Consolidate multiple InvalidateDrawable() calls into one

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@naveenkumar-sanjeevirayan

Copy link
Copy Markdown
Collaborator

Tab View and Text Input Layout changes are fine

@Karthickmani97 Karthickmani97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shimmer control changes are fine

@AmalRajUmapathySelvam AmalRajUmapathySelvam left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Popup changes are fine.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants