Repository navigation
Commit 1afd07f
Fix device placement for memory-planned buffers in Runtime.load_program (pytorch#22058)
# Fix device placement for memory-planned buffers in
Runtime.load_program
Replaces pytorch#22057. That pull request was force-pushed to a commit with no
history in common with main, which made GitHub close it permanently.
Same
branch, same change, correct history.
## The problem
ExecuTorch has two ways to load a model from Python. With the CUDA
backend, one
of them works and the other crashes. Same `.pte` file, same `.ptd`
weights file,
same default export settings. Only the loader differs.
```python
# works
_load_for_executorch(pte_path, ptd_path).forward([x])
# crashes
Runtime.get().load_program(pte_path, data_path=ptd_path) \
.load_method("forward").execute([x])
```
The crash looks like this:
```
[cuda_backend.cpp:548] Tensor 0 has device_type=CUDA but its data pointer
0x... is not backed by CUDA device memory
(cudaPointerGetAttributes err=0, cudaMemoryType=0).
RuntimeError: method->execute() failed with error 0x12
```
## Why it happens
A `.pte` file records where each memory-planned buffer has to live, on
the host
or on an accelerator. That is the `non_const_buffer_device` field in the
plan.
`ProgramMemory` in `extension/pybindings/pybindings.cpp` never read that
field.
It allocated every planned buffer as host memory
(`std::vector<uint8_t>`). So a
program that asked for device memory received a host pointer. The CUDA
backend
checked the pointer, saw it was not device memory, and refused to run.
```
.pte says: buffer 0 -> CUDA:0
before: buffer 0 -> std::vector<uint8_t> (host) -> backend rejects
after: buffer 0 -> DeviceMemoryBuffer::create (device) -> backend accepts
```
## The fix
`extension/module/module.cpp` hits the same problem in the C++ `Module`
API and
answers it differently, because it has a `share_memory_arenas` flag and
this
loader has none. `Module` refuses to load a device-planned method when
that flag
is set, and otherwise builds per-method arenas for every method.
`Runtime.load_program` shares arenas unconditionally today and offers no
way to
turn that off, so refusing would stop files loading that load now. It
keeps the
shared host arenas for host methods and gives a device-planned method
its own.
1. `ProgramMemory` now receives the per-buffer device list alongside the
sizes,
and allocates each buffer on the device it is tagged for. Host-tagged
buffers
keep using `std::vector<uint8_t>`, exactly as before.
2. Buffer indices are plan local. Buffer 0 of one method has nothing to
do with
buffer 0 of another, so one set of arenas shared by index cannot
describe a
file where one method plans onto the host and another onto an
accelerator. A
method with any device-tagged buffer therefore gets its own arenas, and
every
other method keeps using the shared host arenas.
3. Those arenas are built in `load_method`, not when the program is
loaded. A
file containing one accelerator method still opens on a machine without
that
accelerator, still lists its methods, and its host methods still load
and
run. Only loading the accelerator method fails, and it fails naming the
buffer and the device it could not allocate.
4. The caller reads the device for each buffer through
`MethodMeta::memory_planned_buffer_device`.
5. Two deprecated calls were replaced with their current names. Both are
plain
forwarders, so behavior is unchanged.
`Program.load_method` in `runtime/__init__.py` documents all of this,
including
the part that is not new: two host-only methods of the same program
share one
set of arenas and therefore overwrite each other's intermediate values.
## Existing programs are unaffected
`non_const_buffer_device` is optional.
`MethodMeta::memory_planned_buffer_device`
returns `Device{CPU, 0}` when the field is absent, which is the case for
CPU-only programs and for `.pte` files produced before the field
existed. Such a
program keeps the shared arenas, the host allocation path, and the
single
argument `HierarchicalAllocator`, so
`MemoryManager::has_device_memory()` stays
false for it as that constructor documents.
## Test plan
Two tests in `extension/pybindings/test/test_pybindings.py`.
**`test_program_loads_when_one_method_is_device_planned`** covers the
refusal
path and needs no GPU, so it runs in the existing CPU-only job. It
exports a two
method program where one method has a device-tagged planned buffer and
the other
has none, then checks that the program loads, that the host method runs,
that
loading the device method reaches the device allocator and is refused
there, and
that the host method still runs afterwards. It first asserts that the
exported
program really does carry a CUDA-tagged planned buffer, so it cannot
quietly
degrade into a plain multi-method test if planning stops tagging
devices. It
skips itself on a build that links the CUDA backend, because that
registers a
CUDA allocator at static init, the registry has no way to drop one, and
the
request would then be satisfied with or without this change.
**`test_device_planned_method_allocates_on_the_device`** covers what the
refusal
is protecting: on a build that does have a device allocator, the arena
has to
come off the device rather than out of host memory. It builds a two
method
program where one method is lowered to the CUDA backend and the other is
not,
then measures free device memory with `torch.cuda.mem_get_info` around
each
`load_method` call. It asserts that loading the host method takes no
device
memory, that loading the device method takes at least 90 percent of the
planned
device bytes, that both methods return correct numbers, that running the
host
method in between does not disturb the device method, and that the
memory is
returned once the methods and the program are all dropped. The test
deletes both
methods before releasing the program: each method owns its own memory,
so a method
kept alive after the program is released still holds its allocation. It
skips
unless the build links the CUDA backend and a device is visible.
That second test can only run where a device allocator is registered, so
this
pull request also wires it into the job that has one. The
`unittest-cuda` job in
`.github/workflows/cuda.yml` runs it, right after the install that job
already
does and before its builds, since the test needs nothing they produce.
That
workflow now triggers on changes under `extension/pybindings/` and to
`runtime/__init__.py`, and the job condition lists the same two paths so
the job
actually fires on them. Before this, no job in the repository built the
Python
extension with a device allocator and then ran the pybindings tests,
which is
exactly why this class of defect was invisible.
### Measured
Linux x86_64, NVIDIA A100 80GB, compute capability 8.0, CUDA 13.0,
Python 3.12,
torch 2.13.0. Two builds from one source tree, one CPU only and one with
the
CUDA backend. The before column is the merge base with `main`, produced
by
swapping only `pybindings.cpp` and rebuilding, so both columns are the
same
machine, the same model files and the same everything else.
Two methods in one program, `forward` on the host and `forward2` lowered
to
CUDA, planned device bytes 50331648 (48 MiB):
| Measurement | Before | After |
| --- | --- | --- |
| Device memory taken by `load_program` | 0 MiB | 0 MiB |
| Device memory taken by loading the host method | 0 MiB | 0 MiB |
| Device memory taken by loading the CUDA method | 0 MiB | 48 MiB |
| CUDA method produces correct numbers | no, error 0x12 | yes |
| Host method produces correct numbers | yes | yes |
| CUDA method still correct after running the host method | not reached
| yes |
| Device memory returned once the methods and program are dropped |
nothing to return | 48 MiB |
The before column is not a generic failure. The CUDA backend names the
defect
itself:
```
[cuda_backend.cpp:548] Tensor 0 has device_type=CUDA but its data pointer
0x7f5842fff010 is not backed by CUDA device memory
(cudaPointerGetAttributes err=0, cudaMemoryType=0).
[method.cpp:1528] CALL_DELEGATE execute failed at instruction 2: 0x12
```
Suites, on the same two builds:
| Suite | CUDA build | CPU-only build |
| --- | --- | --- |
| `extension/pybindings/test/test_pybindings.py` | 39 passed, 1 skipped,
2 failed | 39 passed, 1 skipped, 2 failed |
| the same file filtered to `-k device` | 4 passed, refusal test skipped
| 4 passed, success test skipped |
| `test_device_planned_method_allocates_on_the_device` alone, run the
way the CI script runs it | passed | skipped |
The two failures are `test_method_quantized_ops` and
`test_quantized_ops`. They
are pre-existing and unrelated: they need the quantized AOT library
preloaded,
which the Buck target does and a bare `pytest` invocation does not. They
reproduce identically at the merge base.
Linux aarch64, Jetson Orin Nano, Python 3.10, CPU-only build: identical
counts to
the x86_64 CPU-only column above, including the device tests and the
same two
pre-existing quantized-op failures. The CUDA success path is not
reachable on
that board, because its GPU needs a PyTorch build pinned to a different
version
than this repository requires, so only the host paths and the refusal
path are
covered there.
## Landing order
This should land after or together with pytorch#22095. The device arenas added
here are
allocated through the device allocator, and ETDump can be handed
pointers into
them when a delegate logs its arguments. `BufferDataSink::write` does a
plain host
`memcpy`, so recording a CUDA tensor would read device memory from the
host.
pytorch#22095 is the fix for that path: it routes non-CPU tensors through a
device copy
before writing. Landing this one first leaves that combination reachable
whenever
event tracing is on.
## Not covered
- Leaving a device-planned method out of the shared host arenas is not
covered
by any test. Delete that skip and both tests still pass, because nothing
in
Python can observe the shared arena sizes. What it saves is host memory
that
nothing reads, which grows with the model, so it is worth a C++ test
later.
- The device path is measured on one accelerator, an A100 with compute
capability 8.0. Not measured on a Jetson board or on any non-CUDA
accelerator.
- `has_device_buffers` and `make_method_memory` ask `MethodMeta` for one
buffer
at a time, and `MethodMeta::memory_planned_buffer_device` scans the
sparse
device list on each call, so the cost is the buffer count times the
device
entry count. Real programs measured here have 2 or 3 buffers and 1
device
entry, and `extension/module/module.cpp` already reads the same metadata
the
same way, but both counts come from the file. Removing the concern
properly
means a bulk accessor on `MethodMeta`, which would fix both callers at
once
and belongs in its own change.
---------
Co-authored-by: Anthony Shoumikhin <shoumikhin@users.noreply.github.com>
Co-authored-by: r <r@e>1 parent 7d163c9 commit 1afd07f
7 files changed
Lines changed: 399 additions & 21 deletions
File tree
- .github/workflows
- extension/pybindings
- test
- runtime
- shim_et/xplat/executorch/extension/pybindings
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
| 29 | + | |
| 30 | + | |
29 | 31 | | |
30 | 32 | | |
31 | 33 | | |
| |||
160 | 162 | | |
161 | 163 | | |
162 | 164 | | |
| 165 | + | |
| 166 | + | |
163 | 167 | | |
164 | 168 | | |
165 | 169 | | |
166 | 170 | | |
167 | 171 | | |
168 | 172 | | |
169 | 173 | | |
| 174 | + | |
| 175 | + | |
170 | 176 | | |
171 | 177 | | |
172 | 178 | | |
| |||
190 | 196 | | |
191 | 197 | | |
192 | 198 | | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
193 | 222 | | |
194 | 223 | | |
195 | 224 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
10 | 11 | | |
11 | 12 | | |
12 | 13 | | |
| |||
31 | 32 | | |
32 | 33 | | |
33 | 34 | | |
| 35 | + | |
34 | 36 | | |
35 | 37 | | |
36 | 38 | | |
| |||
81 | 83 | | |
82 | 84 | | |
83 | 85 | | |
| 86 | + | |
84 | 87 | | |
85 | 88 | | |
86 | 89 | | |
87 | 90 | | |
88 | 91 | | |
89 | 92 | | |
90 | 93 | | |
| 94 | + | |
91 | 95 | | |
92 | 96 | | |
93 | 97 | | |
| |||
1077 | 1081 | | |
1078 | 1082 | | |
1079 | 1083 | | |
1080 | | - | |
| 1084 | + | |
| 1085 | + | |
| 1086 | + | |
| 1087 | + | |
| 1088 | + | |
| 1089 | + | |
| 1090 | + | |
| 1091 | + | |
| 1092 | + | |
| 1093 | + | |
| 1094 | + | |
1081 | 1095 | | |
1082 | | - | |
| 1096 | + | |
| 1097 | + | |
| 1098 | + | |
| 1099 | + | |
1083 | 1100 | | |
1084 | | - | |
1085 | | - | |
| 1101 | + | |
1086 | 1102 | | |
1087 | 1103 | | |
1088 | 1104 | | |
1089 | 1105 | | |
1090 | 1106 | | |
1091 | 1107 | | |
| 1108 | + | |
| 1109 | + | |
| 1110 | + | |
1092 | 1111 | | |
1093 | 1112 | | |
1094 | 1113 | | |
| |||
1105 | 1124 | | |
1106 | 1125 | | |
1107 | 1126 | | |
| 1127 | + | |
| 1128 | + | |
| 1129 | + | |
| 1130 | + | |
| 1131 | + | |
| 1132 | + | |
| 1133 | + | |
| 1134 | + | |
| 1135 | + | |
| 1136 | + | |
1108 | 1137 | | |
1109 | 1138 | | |
1110 | 1139 | | |
| |||
1113 | 1142 | | |
1114 | 1143 | | |
1115 | 1144 | | |
| 1145 | + | |
| 1146 | + | |
| 1147 | + | |
| 1148 | + | |
| 1149 | + | |
| 1150 | + | |
| 1151 | + | |
| 1152 | + | |
| 1153 | + | |
| 1154 | + | |
| 1155 | + | |
| 1156 | + | |
| 1157 | + | |
| 1158 | + | |
| 1159 | + | |
| 1160 | + | |
| 1161 | + | |
| 1162 | + | |
| 1163 | + | |
| 1164 | + | |
| 1165 | + | |
| 1166 | + | |
| 1167 | + | |
| 1168 | + | |
| 1169 | + | |
| 1170 | + | |
| 1171 | + | |
| 1172 | + | |
| 1173 | + | |
| 1174 | + | |
| 1175 | + | |
| 1176 | + | |
| 1177 | + | |
| 1178 | + | |
| 1179 | + | |
| 1180 | + | |
| 1181 | + | |
| 1182 | + | |
| 1183 | + | |
| 1184 | + | |
| 1185 | + | |
| 1186 | + | |
| 1187 | + | |
| 1188 | + | |
| 1189 | + | |
| 1190 | + | |
| 1191 | + | |
| 1192 | + | |
| 1193 | + | |
| 1194 | + | |
| 1195 | + | |
| 1196 | + | |
| 1197 | + | |
| 1198 | + | |
1116 | 1199 | | |
1117 | 1200 | | |
1118 | | - | |
1119 | | - | |
1120 | | - | |
| 1201 | + | |
| 1202 | + | |
| 1203 | + | |
| 1204 | + | |
| 1205 | + | |
| 1206 | + | |
| 1207 | + | |
| 1208 | + | |
1121 | 1209 | | |
1122 | 1210 | | |
1123 | 1211 | | |
| 1212 | + | |
| 1213 | + | |
| 1214 | + | |
| 1215 | + | |
| 1216 | + | |
| 1217 | + | |
| 1218 | + | |
| 1219 | + | |
| 1220 | + | |
1124 | 1221 | | |
1125 | 1222 | | |
| 1223 | + | |
| 1224 | + | |
| 1225 | + | |
| 1226 | + | |
| 1227 | + | |
| 1228 | + | |
| 1229 | + | |
| 1230 | + | |
| 1231 | + | |
| 1232 | + | |
| 1233 | + | |
| 1234 | + | |
| 1235 | + | |
| 1236 | + | |
| 1237 | + | |
| 1238 | + | |
| 1239 | + | |
| 1240 | + | |
| 1241 | + | |
| 1242 | + | |
| 1243 | + | |
| 1244 | + | |
| 1245 | + | |
| 1246 | + | |
| 1247 | + | |
| 1248 | + | |
| 1249 | + | |
| 1250 | + | |
| 1251 | + | |
| 1252 | + | |
| 1253 | + | |
| 1254 | + | |
| 1255 | + | |
| 1256 | + | |
| 1257 | + | |
| 1258 | + | |
| 1259 | + | |
| 1260 | + | |
| 1261 | + | |
| 1262 | + | |
| 1263 | + | |
1126 | 1264 | | |
1127 | 1265 | | |
1128 | 1266 | | |
| |||
1423 | 1561 | | |
1424 | 1562 | | |
1425 | 1563 | | |
1426 | | - | |
1427 | | - | |
| 1564 | + | |
| 1565 | + | |
| 1566 | + | |
| 1567 | + | |
| 1568 | + | |
| 1569 | + | |
| 1570 | + | |
| 1571 | + | |
| 1572 | + | |
| 1573 | + | |
| 1574 | + | |
1428 | 1575 | | |
1429 | 1576 | | |
1430 | 1577 | | |
| |||
1434 | 1581 | | |
1435 | 1582 | | |
1436 | 1583 | | |
1437 | | - | |
1438 | | - | |
1439 | | - | |
1440 | | - | |
1441 | | - | |
1442 | | - | |
1443 | | - | |
| 1584 | + | |
| 1585 | + | |
| 1586 | + | |
| 1587 | + | |
| 1588 | + | |
1444 | 1589 | | |
1445 | 1590 | | |
1446 | | - | |
| 1591 | + | |
1447 | 1592 | | |
1448 | 1593 | | |
1449 | 1594 | | |
| |||
1508 | 1653 | | |
1509 | 1654 | | |
1510 | 1655 | | |
| 1656 | + | |
| 1657 | + | |
| 1658 | + | |
| 1659 | + | |
| 1660 | + | |
| 1661 | + | |
| 1662 | + | |
| 1663 | + | |
| 1664 | + | |
| 1665 | + | |
| 1666 | + | |
| 1667 | + | |
| 1668 | + | |
1511 | 1669 | | |
1512 | 1670 | | |
1513 | | - | |
| 1671 | + | |
1514 | 1672 | | |
1515 | 1673 | | |
1516 | 1674 | | |
| |||
1519 | 1677 | | |
1520 | 1678 | | |
1521 | 1679 | | |
1522 | | - | |
| 1680 | + | |
| 1681 | + | |
| 1682 | + | |
1523 | 1683 | | |
1524 | 1684 | | |
1525 | 1685 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
| 29 | + | |
29 | 30 | | |
30 | 31 | | |
31 | 32 | | |
| |||
39 | 40 | | |
40 | 41 | | |
41 | 42 | | |
| 43 | + | |
| 44 | + | |
42 | 45 | | |
43 | 46 | | |
44 | 47 | | |
| |||
50 | 53 | | |
51 | 54 | | |
52 | 55 | | |
| 56 | + | |
| 57 | + | |
53 | 58 | | |
54 | 59 | | |
55 | 60 | | |
| |||
61 | 66 | | |
62 | 67 | | |
63 | 68 | | |
| 69 | + | |
| 70 | + | |
64 | 71 | | |
65 | 72 | | |
66 | 73 | | |
| |||
0 commit comments