Repository navigation
Conversation
The copying constructor allocated its buffer and then called `*this = other`, which resolves to arena_matrix's own templated operator= and allocates a second buffer. The first one was never used, so every arena copy took twice the memory. Assign into the buffer with Base::operator= instead.
Jenkins Console Log Machine informationDistributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focal CPU: Architecture: x86_64 CPU op-mode(s): 32-bit, 64-bit Byte Order: Little Endian Address sizes: 43 bits physical, 48 bits virtual CPU(s): 256 On-line CPU(s) list: 0-255 Thread(s) per core: 2 Core(s) per socket: 64 Socket(s): 2 NUMA node(s): 2 Vendor ID: AuthenticAMD CPU family: 23 Model: 49 Model name: AMD EPYC 7742 64-Core Processor Stepping: 0 Frequency boost: enabled CPU MHz: 1386.314 CPU max MHz: 3416.0681 CPU min MHz: 1500.0000 BogoMIPS: 4491.56 Virtualization: AMD-V L1d cache: 4 MiB L1i cache: 4 MiB L2 cache: 64 MiB L3 cache: 512 MiB NUMA node0 CPU(s): 0-63,128-191 NUMA node1 CPU(s): 64-127,192-255 Vulnerability Gather data sampling: Not affected Vulnerability Indirect target selection: Not affected Vulnerability Itlb multihit: Not affected Vulnerability L1tf: Not affected Vulnerability Mds: Not affected Vulnerability Meltdown: Not affected Vulnerability Mmio stale data: Not affected Vulnerability Old microcode: Not affected Vulnerability Reg file data sampling: Not affected Vulnerability Retbleed: Mitigation; untrained return thunk; SMT enabled with STIBP protection Vulnerability Spec rstack overflow: Mitigation; Safe RET Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl Vulnerability Spectre v1: Mitigation; usercopy/swapgs barriers and __user pointer sanitization Vulnerability Spectre v2: Mitigation; Retpolines; IBPB conditional; STIBP always-on; RSB filling; PBRSB-eIBRS Not affected; BHI Not affected Vulnerability Srbds: Not affected Vulnerability Tsa: Not affected Vulnerability Tsx async abort: Not affected Vulnerability Vmscape: Mitigation; IBPB before exit to userspace Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology nonstop_tsc cpuid extd_apicid aperfmperf rapl pni pclmulqdq monitor ssse3 fma cx16 sse4_1 sse4_2 x2apic movbe popcnt aes xsave avx f16c rdrand lahf_lm cmp_legacy svm extapic cr8_legacy abm sse4a misalignsse 3dnowprefetch osvw ibs skinit wdt tce topoext perfctr_core perfctr_nb bpext perfctr_llc mwaitx cpb cat_l3 cdp_l3 hw_pstate ssbd mba ibrs ibpb stibp vmmcall fsgsbase bmi1 avx2 smep bmi2 cqm rdt_a rdseed adx smap clflushopt clwb sha_ni xsaveopt xsavec xgetbv1 xsaves cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local clzero irperf xsaveerptr rdpru wbnoinvd amd_ppin arat npt lbrv svm_lock nrip_save tsc_scale vmcb_clean flushbyasid decodeassists pausefilter pfthreshold avic v_vmsave_vmload vgif v_spec_ctrl umip rdpid overflow_recov succor smca sev sev_es G++: g++ (Ubuntu 9.4.0-1ubuntu1~20.04) 9.4.0 Copyright (C) 2019 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Clang: clang version 10.0.0-4ubuntu1 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin |
| char* before = static_cast<char*>(memalloc.alloc(8)); | ||
| arena_matrix<Eigen::VectorXd> a(x); | ||
| char* after = static_cast<char*>(memalloc.alloc(8)); |
There was a problem hiding this comment.
You can just ask for memalloc.bytes_allocated() before and after the allocation
There was a problem hiding this comment.
bytes_allocated() only counts whole blocks, so it doesn't change for a mere 40-byte allocation inside the current one. i.e. the test would pass on current develop without this patch.
There was a problem hiding this comment.
Ah sorry I just looked at the function names. Could you add something like this to the stack allocator and use it in the test
inline size_t approx_bytes_used() const {
size_t sum = 0;
for (size_t i = 0; i < cur_block_; ++i) {
sum += sizes_[i];
}
return sum + static_cast<size_t>(next_loc_ - blocks_[cur_block_]);
}|
Thanks! Besides the note above to simplify the test a bit I think this is good |
|
@SteveBronder I believe your simplification is not applicable here because of the amount of tested memory being too small, see the comment above. |
A one-liner that as has been eating up significant amount of autodiff memory since 2020
Summary
arena_matrix's constructor from an Eigen expression or matrix allocates its buffer twice:Inside the constructor,
*this = otherresolves toarena_matrix's own templatedoperator=(const T&), an exact match. That operator places the map onto a second arena allocation of the same size and copies into it, so the first buffer is never used. The pattern dates back to f94d6f1 (#1970).Every copy onto the arena that goes through this constructor therefore took twice the memory: 16N bytes instead of 8N for
doubleorvar. That includes:to_arenaof any Eigen argument that is not already anarena_matrix, rvalues included;arena_t<T> x = expr;for any expression, and for any matrix except a plain rvalue of exactly the matrix type, which is moved instead;var_value<Matrix>constructed from a value that is not anarena_matrix. Value and adjoint together drop from 3N to 2N doubles, and from 4N to 2N when both are given.Copies of the same
arena_matrixtype and of exactlyMap<MatrixType>were not affected.The fix assigns into the buffer the constructor has already allocated:
Base::operator=(other);This is the same call
operator=makes after allocating. It uses the sameget_rows/get_cols, so orientation,double→varconversion and empty inputs behave as before. Only the target buffer differs.Tests
Added a test which fails on current develop. Mostly meant to document the issue.
Side Effects
Pure gain.
Some experiments showing that this is quite significant:
The arena bytes per call are measured between two sentinel allocations. At N = 1024 a gradient takes:
normal_lpdf(y, mu, sigma)with avarvectormusum(elt_multiply(a, b))sum(multiply(X, beta)), X is N × 5 datadot_product(a, b)In
normal_lpdf, half of the saving comes from the copy ofmuand half from its zero-filled partials.Speed change within noise.
Release notes
Reduced autodiff memory use.
Checklist
Copyright holder: Jáchym Barvínek [email protected]
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit)make test-headers)make test-math-dependencies)make doxygen)make cpplint)the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested