fix(android): scrollview optimisation#14425
Conversation
- Throttle scroll events to ~60fps (16ms) in both vertical and horizontal onScrollChanged() - Cache contentSize() TiDimension values (only recalculate when dimensions change) - Cache contentWidth/contentHeight in TiScrollViewLayout.getContentProperty() per measure pass - Invalidate content property cache on property changes and at start of onMeasure() - Lazy-create TiSwipeRefreshLayout only when refreshControl property is set - Add createSwipeRefreshLayout() helper for dynamic wrapper creation - Add optimization plan document
…I.ScrollView - Remove getScrollY() > 0 workaround in scrollTo() — NestedScrollView bug fixed in AndroidX - Fix scrollToTop() animated path: use smoothScrollTo(0,0) instead of smoothScrollBy hack - Move initial content offset from onDraw() to onLayout() for earlier pipeline application - Update optimization plan with Phase 4 completion status
2573efe to
020ad65
Compare
|
Very nice stats 👍 Don't think I have a heave scrollview app somewhere to test it but I'll definitely check it out. |
|
Excellent work @mbender74! Good to see your work again! |
hansemannn
left a comment
There was a problem hiding this comment.
Summary
Solid optimization pass — throttling, caching, and lazy TiSwipeRefreshLayout all look reasonable. One high-severity correctness regression blocks ship, though.
Blocking: dragstart / dragend can be silently dropped
In both TiVerticalScrollView.onScrollChanged (around line 537) and TiHorizontalScrollView.onScrollChanged (around line 685), the 16 ms throttle's return sits above the dragstart block:
super.onScrollChanged(l, t, oldl, oldt);
// Throttle scroll events to ~60fps
long currentTime = SystemClock.elapsedRealtime();
if ((currentTime - lastScrollEventTime) < SCROLL_EVENT_THROTTLE_MS) {
return; // <-- skips dragstart logic
}
lastScrollEventTime = currentTime;
if (!isScrolling && isTouching) {
isScrolling = true;
...
getProxy().fireEvent(TiC.EVENT_DRAGSTART, data);
}This contradicts the PR description ("Do NOT throttle dragstart/dragend events"), and because isScrolling is only set to true inside that throttled block, while dragend in onTouchEvent is gated on isScrolling being true, a single suppressed dragstart causes the matching dragend to also never fire.
Concrete repro path:
- Anything updates
lastScrollEventTime— a programmaticscrollTo, fling settle, or the tail of a previous gesture. - Within 16 ms, the user starts a brief drag whose first
onScrollChangedlands inside the window. - Throttle returns early → no
dragstart,isScrollingstaysfalse. ACTION_UParrives →isScrollingis stillfalse→ nodragend.
Net result: a real user drag fires neither dragstart nor dragend. JS event listeners that pair these (e.g. setting/clearing a "user is dragging" flag, snap-on-release logic) will be left in an inconsistent state.
Suggested fix
Move the dragstart block above the throttle check, so only the high-frequency scroll event is throttled:
super.onScrollChanged(l, t, oldl, oldt);
// Always fire dragstart immediately — must not be throttled,
// otherwise the matching dragend (gated on isScrolling) is also lost.
if (!isScrolling && isTouching) {
isScrolling = true;
KrollDict startData = new KrollDict();
startData.put(TiC.EVENT_PROPERTY_X, xDimension.getAsDefault(scrollView));
startData.put(TiC.EVENT_PROPERTY_Y, yDimension.getAsDefault(scrollView));
getProxy().fireEvent(TiC.EVENT_DRAGSTART, startData);
}
// Throttle the high-frequency scroll event only.
long currentTime = SystemClock.elapsedRealtime();
if ((currentTime - lastScrollEventTime) < SCROLL_EVENT_THROTTLE_MS) {
return;
}
lastScrollEventTime = currentTime;
setContentOffset(l, t);
// ... fire EVENT_SCROLLApply the same reordering to the horizontal variant. Worth adding a regression test that does a programmatic scrollTo followed immediately by a synthesized touch drag and asserts dragstart/dragend both fire.
|
@hansemannn changes made, your review was right |
hansemannn
left a comment
There was a problem hiding this comment.
Looking good now, thank you!
ScrollView Android Optimization Plan
Overview
This document tracks all identified performance issues and planned optimizations
for the Android Ti.UI.ScrollView implementation.
Baseline: NestedScrollView (vertical) / HorizontalScrollView (horizontal), no RecyclerView,
no view recycling, no hardware layer optimization
Identified Performance Bottlenecks
BOTTLENECK #1: Unthrottled Scroll Events (CRITICAL)
Problem:
onScrollChanged()fires ascrollevent to JavaScript on every pixel change.Per event: 1
KrollDict+ 2TiDimension(fromcontentSize()) + 2TiDimension(from
setContentOffset()) + JNI crossing + JS execution. That's ~6 object allocationsper frame at 60fps = 360 objects/second.
File:
TiUIScrollView.javaline 498 (vertical), line 633 (horizontal)Fix:
SCROLL_EVENT_THROTTLE_MS = 16constant (~60fps)lastScrollEventTimefieldonScrollChanged(): only fire whenSystemClock.elapsedRealtime() - lastScrollEventTime > THROTTLE_MSdragstart/dragendevents (fired only once)Expected improvement: 40-50% less event overhead during scrolling
Status: DONE — 16ms throttling implemented in both
onScrollChanged()methodsBOTTLENECK #2: No View Recycling (CRITICAL for large content)
Problem:
NestedScrollView/HorizontalScrollViewhold ALL child views in memory andmeasure/layout them on every layout pass, regardless of visibility. O(n) measure/layout
where n = all children, not just visible ones.
File: Architecture decision —
TiUIScrollView.javaFix Option A (Minimal): Enable hardware layers for off-screen children
View.LAYER_TYPE_HARDWARELAYER_TYPE_NONEonScrollChanged()callback checking viewport boundsFix Option B (Medium): View-Stub pattern for complex children
Note: Migrating to RecyclerView is a breaking change and requires
API changes (contentWidth/contentHeight behavior). Not recommended for Phase 1.
Expected improvement (Option A): 15-20% less measure/layout time
Expected improvement (Option B): 30-40% less memory and measure time
Status: SKIPPED — Hardware-layer viewport tracking is complex and error-prone
for the NestedScrollView architecture. Ti.UI.ListView should be used for large content sets.
ScrollView remains optimized for small to medium content.
BOTTLENECK #3: contentWidth/contentHeight Not Cached (MEDIUM)
Problem:
getContentProperty()is called up to 5x per measure pass(in
onMeasure(),getMeasuredWidth(),getMeasuredHeight(),getWidthMeasureSpec(),getHeightMeasureSpec()). Each call does a proxy property lookup, string comparisons,and potentially creates
TiDimensionobjects.File:
TiUIScrollView.javalines 217-243 (TiScrollViewLayout inner class)Fix:
cachedContentWidthValueandcachedContentHeightValuefieldscontentWidthCached/contentHeightCachedboolean flagsgetContentProperty(): check cache before proxy lookup, cache values after computationinvalidateContentPropertyCache()method for cache invalidationonMeasure()and onpropertyChanged()for contentWidth/contentHeightExpected improvement: 20-25% faster measure passes
Status: DONE — contentWidth/contentHeight caching with cache invalidation implemented
BOTTLENECK #4: TiDimension Allocations in Scroll Events (MEDIUM)
Problem:
contentSize()andsetContentOffset()create new TiDimension objects on EVERYscroll event. At 60fps that's 4 TiDimension + 2 KrollDict per frame.
File:
TiUIScrollView.javalines 493-497, 628-632, 717-725, 1059-1070Fix:
contentSize()only recalculate when content size has changedcachedContentWidth/cachedContentHeightint fields for getLayout().getMeasuredWidth/HeightcachedContentSizeWidth/cachedContentSizeHeightdouble fields for TiDimension valuesExpected improvement: 50% fewer allocations during scrolling
Status: DONE — contentSize() caches TiDimension values and only recreates them when dimensions change
BOTTLENECK #5: TiSwipeRefreshLayout Wrapper Overhead (LOW)
Problem: Every ScrollView is wrapped in
TiSwipeRefreshLayout, even whenpull-to-refresh is not used. Extra ViewGroup level +
onMeasure()iteratesall children for WRAP_CONTENT support.
File:
TiUIScrollView.javalines 899-944Fix:
refreshControlis not set, useNestedScrollView/HorizontalScrollViewdirectly as nativeView (without SwipeRefreshLayout wrapper)
TiSwipeRefreshLayoutwhenrefreshControlproperty is setcreateSwipeRefreshLayout())Expected improvement: 5-10% less measure overhead, 1 fewer ViewGroup level
Status: DONE — Lazy creation in
processProperties()and dynamic creationin
propertyChanged()viacreateSwipeRefreshLayout()helper methodBOTTLENECK #6: Smooth-Scrolling Workarounds (LOW)
Problem:
scrollTo()andscrollToTop()disable smooth scrolling due toNestedScrollView bugs. Programmatic scrolling uses
View.scrollTo()(instant jump).File:
TiUIScrollView.javalines 983-1057Fix:
mLastScrollerYbug in NestedScrollView was fixed in Support Library 28.0.0 /androidx.core:core:1.0.0+ (late 2018). Titanium SDK uses AndroidX, so the fix is included.
getScrollY() > 0workaround inscrollTo()— smooth scrolling now always worksscrollToTop()animated path: usessmoothScrollTo(0, 0)instead ofsmoothScrollBy(0, -height)hack with temporarysetSmoothScrollingEnabled(false)Expected improvement: Smoother programmatic scroll animations
Status: DONE — Removed workarounds, smooth scrolling enabled for all positions
BOTTLENECK #7: onDraw for Initial Offset (LOW)
Problem:
onDraw()is overridden to set the initial content offset.This causes a visible jump from (0,0) to the offset position on the first draw pass.
File:
TiUIScrollView.javalines 469-477, 603-611Fix:
onLayout()instead ofonDraw()(earlier in the pipeline)onDraw()now only callssuper.onDraw()— no offset logiconLayout()to set initial offsetExpected improvement: No visible jump on load
Status: DONE — Initial offset set in onLayout instead of onDraw
BOTTLENECK #8: onLayout Double-Measure in TiCompositeLayout (LOW)
Problem: When pin-based width/height calculation in
onLayout()differs fromonMeasure(),child.measure()is called AGAIN. In ScrollView this cascades through all children.File:
TiCompositeLayout.javaline 922-926Fix:
onMeasure()instead ofonLayout()onLayout()only for positioning, not for re-measurementrequestLayout()instead of directchild.measure()Expected improvement: Reduces double-measure situations
Status: SKIPPED — Too risky for backward compatibility. TiCompositeLayout is used
by many view types, changes could have side effects.
Phase Plan
Phase 1: Scroll Event Optimization (Highest Priority)
Phase 2: Measure/Layout Optimization
Reduce double-measure in TiCompositeLayout(skipped — too risky)Phase 3: View Hierarchy Optimization
Hardware layer for off-screen children (Option A)(skipped)Phase 4: Smooth Scrolling
Code Review Results (Phase 1-3)
BOTTLENECK #1: Scroll Event Throttling
Status: DONE
SCROLL_EVENT_THROTTLE_MS = 16constantlastScrollEventTimefieldonScrollChanged()methods (vertical + horizontal) check throttleSystemClock.elapsedRealtime()for precise timingdragstart/dragendevents are NOT throttledBOTTLENECK #3: contentWidth/contentHeight Caching
Status: DONE
cachedContentWidthValue/cachedContentHeightValueint fields in TiScrollViewLayoutcontentWidthCached/contentHeightCachedboolean flagsgetContentProperty()checks cache before proxy lookup, caches result after computationinvalidateContentPropertyCache()method for cache invalidationonMeasure()propertyChanged()fires for contentWidth/contentHeightBOTTLENECK #4: TiDimension Allocations
Status: DONE
contentSize()caches TiDimension values incachedContentSizeWidth/cachedContentSizeHeightcachedContentWidth/cachedContentHeightint fields for getLayout().getMeasuredWidth/HeightBOTTLENECK #5: Lazy TiSwipeRefreshLayout
Status: DONE
swipeRefreshLayoutfield on TiUIScrollView classcreateSwipeRefreshLayout()helper method extractedprocessProperties(): SwipeRefreshLayout only created whenrefreshControlproperty existsrefreshControl: ScrollView directly as nativeView (setNativeView(this.scrollView))propertyChanged()for REFRESH_CONTROL: Dynamic creation whenswipeRefreshLayout == nullBOTTLENECK #6: Smooth Scrolling Workarounds
Status: DONE
mLastScrollerYbug was fixed in Support Library 28.0.0 / AndroidX core 1.0.0+getScrollY() > 0check that disabled smooth scrolling inscrollTo()scrollToTop()animated path:smoothScrollTo(0, 0)instead ofsmoothScrollBy(0, -height)withsetSmoothScrollingEnabled(false)hackBOTTLENECK #7: Initial Offset in onDraw
Status: DONE
TiVerticalScrollViewandTiHorizontalScrollViewnow overrideonLayout()to set initial offsetonDraw()only callssuper.onDraw()— no offset logicTest Strategy
Integration Tests
Performance Tests
Android Profiler
Backward Compatibility
All changes are backward compatible:
refreshControlis still supportedeven when set after creation (dynamic wrapper creation)
Risks & Known Issues
Risk 1: Scroll Event Throttling
Mitigation: 16ms throttle (~60fps) — users won't notice a difference, but
event consumers that need every pixel change might want the old behavior.
Fallback: introduce a
scrollEventThrottleproperty (like iOS).Risk 2: Hardware-Layer Viewport Tracking
Resolution: SKIPPED — Too complex for the NestedScrollView architecture.
Use Ti.UI.ListView for large content sets.
Risk 3: TiSwipeRefreshLayout Lazy Creation
Mitigation: View hierarchy change only on property change, not during
scrolling.
createSwipeRefreshLayout()method encapsulates creation.Dynamic creation in
propertyChanged()whenswipeRefreshLayout == null.Risk 4: contentPropertyCache and parentContentWidth/Height
Note:
getContentProperty()forLAYOUT_FILLusesthis.parentContentWidth/Heightwhich can change between measure passes. Cache is invalidated in
onMeasure(),ensuring fresh values are computed on each measure pass.
Performance Expectations (Overall)
Done Checklist
Phase 1: Scroll Event Optimization
Phase 2: Measure/Layout Optimization
Double-measure reduction(skipped — too risky for TiCompositeLayout)Phase 3: View Hierarchy Optimization
Hardware-layer viewport tracking(skipped — not practical for NestedScrollView)Phase 4: Smooth Scrolling