Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions .claude/skills/perf-benchmarking/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ description: Measure React Native performance on-device for stream-chat-react-na
How to measure SDK performance for real, against the **SampleApp running on a physical Android device**, using the committed `perf/` toolkit. This skill exists because the methodology is full of traps that produce confidently-wrong numbers; follow the rules and recipes here instead of improvising.

Prerequisites for almost everything below:
- **The installed app is a DEBUG build wired to Metro — check this FIRST, before anything else.** A release build embeds the JS bundle in the APK and ignores Metro entirely, so none of your `src` edits or throwaway instrumentation ever run, AND the Hermes inspector is disabled so CPU profiling can't attach. Symptoms are silent and misleading: the app runs fine, navigation works, but every marker you add stays absent and you waste an hour chasing phantom "stale bundle" / "adb reverse" causes (this happened). Verify with one command — `adb shell run-as io.getstream.reactnative.sampleapp ls >/dev/null 2>&1 && echo DEBUG || echo "RELEASE — rebuild"` (a release build reports "package not debuggable"); or check the flags: `adb shell dumpsys package io.getstream.reactnative.sampleapp | grep flags=` should include `DEBUGGABLE`. If it's release, install debug first: `yarn workspace sampleapp android` (builds, installs over the release app, sets up `adb reverse`). Confirm the device→Metro reverse exists: `adb reverse --list` should show `tcp:8081`.
Comment thread
isekovanic marked this conversation as resolved.
- Metro running: `yarn workspace sampleapp start` (the device pulls the JS bundle from it; **Metro bundles the SDK from `src`** via the `react-native` package.json field, so editing `package/src/**` hot-reloads on relaunch β€” no `yarn build` needed).
- A device/emulator connected: `adb devices` shows one. The SampleApp package is `io.getstream.reactnative.sampleapp`.

Expand All @@ -32,7 +33,7 @@ Good-practice bar for this SDK: **if messages (or the action under test) end up
- "What component re-renders too much / too often" β†’ **React DevTools profile** (`analyze-react-profile.js`).
- Memory / jank / dropped frames β†’ **`android-heap-dump.sh`**.
- A CPU-profile **`--diff` is blind to sub-noise changes** and is polluted by line-number-shift phantom deltas when you diff across a code edit. Do not use it to "prove" a tiny change.
6. **A/B fairly.** Same scenario, same instrumentation, same device, same channel (with enough messages), same swipe count. Only the code under test differs.
6. **A/B fairly β€” same data, same path, multiple runs.** Same scenario, instrumentation, device, channel (with enough messages), and swipe count; only the code under test differs. Crucially, **both builds must traverse the *identical content*** β€” the same images, the same messages, the same items in the same order. Content-dependent costs (image decode, layout, text shaping) otherwise confound the result, and the **tail percentiles (95th/99th) are dominated by the single heaviest item traversed** β€” so a different start position or swipe path makes them meaningless (this bit us once: two gallery runs opened on different images, and the 99th moved 50ms purely from decoding a bigger bitmap, not from the code). Pin the start (e.g. force a fixed gallery index via throwaway instrumentation) and drive a fixed path so each run does identical work. And **run each side β‰₯3Γ— and report the spread** β€” a single run per side cannot separate a real delta from run-to-run noise, especially at the tail; a difference inside the runs' spread is not a result.
7. **Drive the device on real mount signals, not `sleep`.** Android *debug* navigation is slow and variable. Wait on testIDs (`channel-preview-button` β†’ `message-flat-list`), never a fixed sleep after a tap β€” or your swipes land on the list / a half-mounted screen.
8. **Respect git-hands-off.** To A/B a committed change, produce the baseline by editing the file back locally (or `git stash` if it's uncommitted), measure, then restore by re-writing the committed version. **Never** `git commit`, `git revert`, or rewrite history to benchmark.

Expand Down Expand Up @@ -116,7 +117,11 @@ Heed rule 5: if the change is sub-noise, the diff will show **nothing real** (on
### 3) Driving the device β€” scenarios on `scenario-lib.sh`

Scenarios are **thin scripts on top of `perf/scenario-lib.sh`**, which provides device-driving verbs so each scenario stays declarative and waits on real mount signals (never `sleep`). Verbs:
`relaunch` Β· `wait_for <testid> [secs]` Β· `tap_testid <id>` Β· `tap_until <tap_id> <expect_id> [tries]` (tap + retry until the next screen's testID appears β€” taps occasionally don't register) Β· `swipe_up/down/left/right [ms]` Β· `scroll [n] [ms]` Β· `count_testid <prefix>` (uses `grep -o | wc -l`, never `grep -c`).
`relaunch` Β· `wait_for <testid> [secs]` Β· `tap_testid <id>` Β· `tap_until <tap_id> <expect_id> [tries]` (tap + retry until the next screen's testID appears β€” taps occasionally don't register) Β· `tap_text <substr>` / `tap_text_until <substr> <expect_id>` (tap a node by its visible text β€” e.g. open a channel by display name) Β· `tap_header_action [ymax]` / `tap_header_until <expect_id>` (tap the right-most header action when RN flattens its testID away) Β· `swipe_up/down/left/right [ms]` Β· `scroll [n] [ms]` Β· `scroll_down [n] [ms] [settle]` Β· `count_testid <prefix>` (uses `grep -o | wc -l`, never `grep -c`) Β· `wait_for_log <pattern> [secs]` (poll logcat for states the view tree can't see).

**Pagination direction depends on whether the list is inverted.** To load more rows you must scroll toward the list's *growing* end and trigger its `onEndReached`. The **message list is inverted** (newest at bottom, older above) β€” paginate by scrolling **up** (`scroll`, the lib's `swipe_up`). The **Photos & Videos media grid is a normal list** (newest first, older below) β€” paginate by scrolling **down** (`scroll_down`, the lib's `swipe_down`). Use the wrong direction and you scroll against the anchored edge: nothing loads, the loaded set looks capped, and every "scaling" run silently measures the **same N** (this happened β€” preload depths 4/12/25 all yielded 44 assets until the grid was scrolled *down*, which grew it to 165).

**Driving the lib inline runs under zsh, which breaks it.** The scenario scripts have a `#!/usr/bin/env bash` shebang, so `bash perf/drive-*.sh` is fine. But `source perf/scenario-lib.sh` from an interactive tool shell runs under **zsh**, where `"$id[^\"]*"` inside `tap_testid`'s grep is parsed as an array subscript (`bad math expression: operand expected at \`^"'`). Always drive via `bash perf/drive-<name>-scenario.sh …`, never by sourcing the lib into zsh.

The channel scenario (`perf/drive-channel-scenario.sh`) is the reference shape:
```bash
Expand All @@ -139,14 +144,18 @@ echo "rows=$(count_testid message-list-item-)"

## Anti-patterns to avoid (each = a real mistake made; do the fix)

- **Measuring against a release build without checking** β†’ src edits + instrumentation silently never run, the Hermes inspector won't attach, and the failure looks exactly like a stale bundle or a missing `adb reverse` (an hour lost chasing both). β†’ `adb shell run-as <pkg> ls` (or check `dumpsys package … flags=` for `DEBUGGABLE`) as step zero; install debug with `yarn workspace sampleapp android` if needed.
- **Node/Jest microbench of native-touching code** β†’ mocked to ~free, ~14Γ— wrong. β†’ Measure on device.
- **Instrumenting one call site** β†’ undercount; misses dependency callers. β†’ Wrap the native API (Pattern 1a).
- **CPU-profile `--diff` for a tiny / cross-edit change** β†’ phantom line-shift deltas dominate, real signal is sub-noise. β†’ Use counting (Pattern 1).
- **`grep -c` on `uiautomator` XML** β†’ the dump is one line, so it returns 1 regardless. β†’ `grep -oE '…' | wc -l`.
- **`uiautomator dump` mid-scroll-animation** β†’ returns a partial/sparse tree (looked like "1 row" when there were 15). β†’ Dump after the list settles.
- **Paginating the wrong direction for the list's inversion** β†’ scroll against the anchored edge, nothing loads, the loaded set looks capped and every "scaling" run measures the same N. β†’ Inverted list (message list) paginates by scrolling **up** (`scroll`); normal list (media grid) paginates by scrolling **down** (`scroll_down`).
- **Sourcing `scenario-lib.sh` into the tool's zsh shell** β†’ `tap_testid`'s `"$id[^\"]*"` is read as a zsh array subscript β†’ `bad math expression`. β†’ Drive via `bash perf/drive-*.sh`, never `source` into zsh.
- **Fixed `sleep` after a tap, then swipe** β†’ debug nav is slow; swipes hit the list / half-mounted screen. β†’ Wait on `message-flat-list` (Pattern 3).
- **Reading cold first-mount numbers** β†’ dominated by startup/JIT/module-init (~2500ms "mount" is mostly not your code). β†’ Use warm re-opens; never quote the cold mount as the row cost.
- **Claiming "faster"/"slower" from one render sample** β†’ it's within noise. β†’ Lead with the deterministic count; only call timing differences real with N runs + non-overlapping spreads.
- **A/B runs that traverse different content** (different images/messages, or a different start index / swipe path between baseline and after) β†’ content-dependent cost (image decode, layout, text shaping) confounds the delta, and the 95th/99th are set by the single heaviest item traversed, so wins *and* losses are noise. β†’ Pin the start (force a fixed index via throwaway instrumentation) and drive an identical path so both builds do byte-identical work; run each side β‰₯3Γ— and compare spreads, not single numbers.

## Environment gotchas

Expand All @@ -167,7 +176,8 @@ The `[SR_STACK]` traces revealed the residual ~30 wasn't ours: 1 call = our prov

## Execution checklist

- [ ] Metro running, exactly one device connected (`adb devices`).
- [ ] **DEBUG build installed** (`adb shell run-as <pkg> ls` succeeds), not release β€” checked BEFORE measuring.
- [ ] Metro running, exactly one device connected (`adb devices`); `adb reverse --list` shows `tcp:8081`.
- [ ] Picked the right **instrument** for the change (rule 5).
- [ ] Chose a channel with enough messages; drove it on testIDs, not sleeps.
- [ ] Instrumented the **native API** (counts every caller), not a single call site.
Expand Down
159 changes: 114 additions & 45 deletions package/src/components/ImageGallery/ImageGallery.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import React, { useCallback, useEffect, useState } from 'react';
import { AccessibilityInfo, ImageStyle, Platform, StyleSheet, ViewStyle } from 'react-native';
import { AccessibilityInfo, ImageStyle, Platform, StyleSheet, View, ViewStyle } from 'react-native';
import { Gesture, GestureDetector } from 'react-native-gesture-handler';

import Animated, {
Easing,
SharedValue,
useAnimatedReaction,
useAnimatedStyle,
useDerivedValue,
Expand Down Expand Up @@ -79,6 +80,109 @@ type ImageGalleryWithContextProps = Pick<
ImageGalleryGrid?: React.ComponentType<ImageGalleryGridProps>;
};

/**
* Number of slides mounted on each side of the current one. Kept one wider
* than AnimatedGalleryImage's +-3 load window so an edge slide is already
* mounted (as the empty placeholder) before it ever needs to load its
* image - paging never reveals an unmounted slot.
*/
const PAGER_WINDOW_RADIUS = 4;

const galleryPagerSelector = (state: ImageGalleryState) => ({
assets: state.assets,
currentIndex: state.currentIndex,
});

type GalleryPagerProps = {
fullWindowHeight: number;
fullWindowWidth: number;
offsetScale: SharedValue<number>;
scale: SharedValue<number>;
slide?: ImageStyle;
translateX: SharedValue<number>;
translateY: SharedValue<number>;
};

/**
* Windowed pager body. Subscribes to `currentIndex` itself (rather than the
* parent doing so) so a page change rerenders only this small slide list and
* never the parent - keeping the gesture objects/`GestureDetector` stable.
*
* Only the slides within +-{@link PAGER_WINDOW_RADIUS} of the current index are
* mounted; a single leading spacer occupies the flex width of all preceding
* slides so the rendered ones keep their exact natural positions. The slide
* transforms in `useAnimatedGalleryStyle` are a pure function of each slide's
* `index` and that natural flex position, so windowing the mount leaves every
* slide pixelidentical while dropping the mounted component/view count to O(window)
* instead of it being O(N). This way, less React fibers are reconciled and less
* shadow nodes are rendered as well.
*/
const GalleryPager = (props: GalleryPagerProps) => {
const { fullWindowHeight, fullWindowWidth, offsetScale, scale, slide, translateX, translateY } =
props;
const { imageGalleryStateStore } = useImageGalleryContext();
const { assets, currentIndex } = useStateStore(
imageGalleryStateStore.state,
galleryPagerSelector,
);

const slideStyle = {
height: fullWindowHeight * 8,
marginRight: MARGIN,
width: fullWindowWidth * 8,
};

const lo = Math.max(0, currentIndex - PAGER_WINDOW_RADIUS);
const hi = Math.min(assets.length - 1, currentIndex + PAGER_WINDOW_RADIUS);

const slides: React.ReactNode[] = [];
for (let i = lo; i <= hi; i++) {
const photo = assets[i];
slides.push(
photo.type === FileTypes.Video ? (
<AnimatedGalleryVideo
attachmentId={photo.id}
index={i}
key={photo.id}
offsetScale={offsetScale}
photo={photo}
scale={scale}
screenHeight={fullWindowHeight}
style={[slideStyle, slide]}
translateX={translateX}
translateY={translateY}
/>
) : (
<AnimatedGalleryImage
accessibilityLabel={'Image Item'}
index={i}
key={photo.id}
offsetScale={offsetScale}
photo={photo}
scale={scale}
screenHeight={fullWindowHeight}
screenWidth={fullWindowWidth}
style={[slideStyle, slide]}
translateX={translateX}
translateY={translateY}
/>
),
);
}

return (
<>
{lo > 0 ? (
<View
key='gallery-pager-leading-spacer'
style={{ width: lo * (fullWindowWidth * 8 + MARGIN) }}
/>
) : null}
{slides}
</>
);
};

export const ImageGalleryWithContext = (props: ImageGalleryWithContextProps) => {
const {
numberOfImageGalleryGridColumns,
Expand Down Expand Up @@ -292,50 +396,15 @@ export const ImageGalleryWithContext = (props: ImageGalleryWithContextProps) =>
testID='image-gallery-pager'
style={[styles.animatedContainer, pagerStyle, pager]}
>
{assets.map((photo, i) =>
photo.type === FileTypes.Video ? (
<AnimatedGalleryVideo
attachmentId={photo.id}
index={i}
key={photo.id}
offsetScale={offsetScale}
photo={photo}
scale={scale}
screenHeight={fullWindowHeight}
style={[
{
height: fullWindowHeight * 8,
marginRight: MARGIN,
width: fullWindowWidth * 8,
},
slide,
]}
translateX={translateX}
translateY={translateY}
/>
) : (
<AnimatedGalleryImage
accessibilityLabel={'Image Item'}
index={i}
key={photo.id}
offsetScale={offsetScale}
photo={photo}
scale={scale}
screenHeight={fullWindowHeight}
screenWidth={fullWindowWidth}
style={[
{
height: fullWindowHeight * 8,
marginRight: MARGIN,
width: fullWindowWidth * 8,
},
slide,
]}
translateX={translateX}
translateY={translateY}
/>
),
)}
<GalleryPager
fullWindowHeight={fullWindowHeight}
fullWindowWidth={fullWindowWidth}
offsetScale={offsetScale}
scale={scale}
slide={slide}
translateX={translateX}
translateY={translateY}
/>
</Animated.View>
</Animated.View>
</GestureDetector>
Expand Down
Loading
Loading