Commit a95a7c1
fix: fix unload in LLM and ModelHostObject to properly free LLM memory (#954)
## Description
Fixes two related memory management bugs in the LLM delete flow:
- **LLM.cpp**. `LLM::unload()` was destroying the runner object but
never calling `BaseModel::unload()` and thus not releasing the module
memory. `LLM::unload()` now calls `BaseModel::unload()` after resetting
the runner.
- **LLMController.ts**. When `load()` was called on an already-loaded
`LLMController`, the previous native module instance was simply
overwritten without calling `unload()` first. This caused a memory leak.
It now unloads the existing native module before loading a new one.
- **ModelHostObject.h**. Reset JSI external memory pressure to `0` after
`model->unload()` so the JS GC is correctly informed that native memory
has been freed.
### Introduces a breaking change?
- [ ] Yes
- [x] No
### Type of change
- [x] Bug fix (change which fixes an issue)
- [ ] New feature (change which adds functionality)
- [ ] Documentation update (improves or adds clarity to existing
documentation)
- [ ] Other (chores, tests, code style improvements etc.)
### Tested on
- [x] iOS
- [x] Android
### Testing instructions
Use the provided screen (you can simply replace the current LLM app
screen) to reproduce the bug and verify the fix. It lets you load and
unload an LLM and observe memory behavior with `vmmap` / `adb`.
- [x] Run the LLM example app
- [x] Prepare memory monitors
- **iOS**: `xcrun simctl spawn booted launchctl list | grep llm` and
`watch -n 0.1 "vmmap <pid> | tail -12"`
- **Android**: `watch -n 0.1 "adb shell dumpsys meminfo
com.anonymous.llm"`
- [x] Press Load - wait for ready
- [x] Note baseline native memory in profiler
- [x] Press Unload - verify native memory returns to baseline
- [x] Press Load again without restarting - verify no accumulation
- [x] Repeat load/unload 5+ times - confirm no upward drift in native
memory and no crash
- [x] Press Load a few times without pressing Unload -- verify no memory
accumulation
```typescript
import { useEffect, useRef, useState } from 'react';
import { StyleSheet, Text, TouchableOpacity, View } from 'react-native';
import { LLMModule, HAMMER2_1_1_5B_QUANTIZED } from 'react-native-executorch';
export default function LLMScreen() {
const [status, setStatus] = useState('idle');
const llmRef = useRef<LLMModule | null>(null);
useEffect(() => {
llmRef.current = new LLMModule();
return () => {
try {
llmRef.current?.interrupt();
llmRef.current?.delete();
} catch {}
};
}, []);
const handleLoad = async () => {
setStatus('loading...');
try {
await llmRef.current!.load(HAMMER2_1_1_5B_QUANTIZED, (p) =>
setStatus(`loading ${(p * 100).toFixed(0)}%`)
);
setStatus('ready');
} catch (e: any) {
setStatus(`load error: ${e?.message}`);
}
};
const handleUnload = () => {
try {
llmRef.current?.interrupt();
llmRef.current?.delete();
setStatus('unloaded');
} catch (e: any) {
setStatus(`unload error: ${e?.message}`);
}
};
const isLoading = status.startsWith('loading');
const canLoad = !isLoading;
const canUnload = status === 'ready';
return (
<View style={styles.container}>
<Text style={styles.status}>{status}</Text>
<TouchableOpacity
style={[styles.button, !canLoad && styles.disabled]}
onPress={handleLoad}
disabled={!canLoad}
>
<Text style={styles.buttonText}>Load</Text>
</TouchableOpacity>
<TouchableOpacity
style={[styles.button, styles.unload, !canUnload && styles.disabled]}
onPress={handleUnload}
disabled={!canUnload}
>
<Text style={styles.buttonText}>Unload</Text>
</TouchableOpacity>
</View>
);
}
const styles = StyleSheet.create({
container: { flex: 1, alignItems: 'center', justifyContent: 'center', gap: 12 },
status: { fontSize: 16, marginBottom: 8 },
button: { backgroundColor: '#2563eb', paddingHorizontal: 32, paddingVertical: 14, borderRadius: 8 },
unload: { backgroundColor: '#dc2626' },
buttonText: { color: '#fff', fontSize: 16, fontWeight: '600' },
disabled: { opacity: 0.4 },
});
```
### Related issues
#948
### Checklist
- [x] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have updated the documentation accordingly
- [x] My changes generate no new warnings
---------
Co-authored-by: Mateusz Sluszniak <56299341+msluszniak@users.noreply.github.com>1 parent 52756c7 commit a95a7c1
File tree
3 files changed
+9
-1
lines changed- packages/react-native-executorch
- common/rnexecutorch
- host_objects
- models/llm
- src/controllers
3 files changed
+9
-1
lines changedLines changed: 1 addition & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
436 | 436 | | |
437 | 437 | | |
438 | 438 | | |
| 439 | + | |
439 | 440 | | |
440 | 441 | | |
441 | 442 | | |
| |||
Lines changed: 4 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
255 | 255 | | |
256 | 256 | | |
257 | 257 | | |
258 | | - | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
259 | 262 | | |
260 | 263 | | |
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
121 | 121 | | |
122 | 122 | | |
123 | 123 | | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
124 | 128 | | |
125 | 129 | | |
126 | 130 | | |
| |||
0 commit comments