test: add memory test and benchmark utilities for Python#5461
test: add memory test and benchmark utilities for Python#5461wjones127 merged 18 commits intolance-format:mainfrom
Conversation
21d7011 to
12906e5
Compare
|
Here's some example output from running the new benchmark: LD_PRELOAD=$(lance-memtest) pytest python/ci_benchmarks/benchmarks/test_search.py::test_io_mem_basic_btree_search -vJSON output that is uploaded to `bencher.dev`{
"test_io_mem_basic_btree_search[small_strings-none]": {
"read_iops": {
"value": 2
},
"read_bytes": {
"value": 1889000
},
"write_iops": {
"value": 0
},
"write_bytes": {
"value": 0
},
"peak_memory_bytes": {
"value": 3797925
},
"total_allocations": {
"value": 135384
}
},
"test_io_mem_basic_btree_search[small_strings-equal]": {
"read_iops": {
"value": 0
},
"read_bytes": {
"value": 0
},
"write_iops": {
"value": 0
},
"write_bytes": {
"value": 0
},
"peak_memory_bytes": {
"value": 7554855
},
"total_allocations": {
"value": 201832
}
},
"test_io_mem_basic_btree_search[small_strings-not_equal]": {
"read_iops": {
"value": 2
},
"read_bytes": {
"value": 1889000
},
"write_iops": {
"value": 0
},
"write_bytes": {
"value": 0
},
"peak_memory_bytes": {
"value": 7554857
},
"total_allocations": {
"value": 202544
}
},
"test_io_mem_basic_btree_search[small_strings-small_range]": {
"read_iops": {
"value": 0
},
"read_bytes": {
"value": 0
},
"write_iops": {
"value": 0
},
"write_bytes": {
"value": 0
},
"peak_memory_bytes": {
"value": 7554907
},
"total_allocations": {
"value": 202022
}
},
"test_io_mem_basic_btree_search[small_strings-large_in]": {
"read_iops": {
"value": 0
},
"read_bytes": {
"value": 0
},
"write_iops": {
"value": 0
},
"write_bytes": {
"value": 0
},
"peak_memory_bytes": {
"value": 7615317
},
"total_allocations": {
"value": 202555
}
},
"test_io_mem_basic_btree_search[integers-none]": {
"read_iops": {
"value": 1
},
"read_bytes": {
"value": 800000
},
"write_iops": {
"value": 0
},
"write_bytes": {
"value": 0
},
"peak_memory_bytes": {
"value": 3797905
},
"total_allocations": {
"value": 135314
}
},
"test_io_mem_basic_btree_search[integers-equal]": {
"read_iops": {
"value": 0
},
"read_bytes": {
"value": 0
},
"write_iops": {
"value": 0
},
"write_bytes": {
"value": 0
},
"peak_memory_bytes": {
"value": 7554835
},
"total_allocations": {
"value": 201821
}
},
"test_io_mem_basic_btree_search[integers-not_equal]": {
"read_iops": {
"value": 1
},
"read_bytes": {
"value": 800000
},
"write_iops": {
"value": 0
},
"write_bytes": {
"value": 0
},
"peak_memory_bytes": {
"value": 7554837
},
"total_allocations": {
"value": 202474
}
},
"test_io_mem_basic_btree_search[integers-small_range]": {
"read_iops": {
"value": 0
},
"read_bytes": {
"value": 0
},
"write_iops": {
"value": 0
},
"write_bytes": {
"value": 0
},
"peak_memory_bytes": {
"value": 7554887
},
"total_allocations": {
"value": 202013
}
},
"test_io_mem_basic_btree_search[integers-large_in]": {
"read_iops": {
"value": 0
},
"read_bytes": {
"value": 0
},
"write_iops": {
"value": 0
},
"write_bytes": {
"value": 0
},
"peak_memory_bytes": {
"value": 7615297
},
"total_allocations": {
"value": 202549
}
}
} |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
westonpace
left a comment
There was a problem hiding this comment.
This is pretty cool. Thanks for coming up with this. We may need a plan for what things we think we want to measure memory usage for and start working on including expected RAM usage in a performance guide as well!
| f"lance-memtest only supports Linux (current platform: {platform.system()}). " | ||
| "Memory statistics will not be available.", | ||
| RuntimeWarning, | ||
| stacklevel=2, |
There was a problem hiding this comment.
I wonder how stacklevel=2 works in a top-level context and not a function call?
| module_dir / "libmemtest.dylib", # macOS | ||
| module_dir / "memtest.dll", # Windows |
There was a problem hiding this comment.
Don't we only support Linux?
| for lib_path in possible_paths: | ||
| if lib_path.exists(): | ||
| lib = ctypes.CDLL(str(lib_path)) | ||
|
|
||
| # Define function signatures | ||
| lib.memtest_get_stats.argtypes = [ctypes.POINTER(_MemtestStats)] | ||
| lib.memtest_get_stats.restype = None | ||
|
|
||
| lib.memtest_reset_stats.argtypes = [] | ||
| lib.memtest_reset_stats.restype = None | ||
|
|
||
| return lib, lib_path | ||
|
|
||
| raise RuntimeError("memtest library not found. Run 'make build' to build it.") |
There was a problem hiding this comment.
Is there any particular reason we can't use pyo3 for the bindings here?
There was a problem hiding this comment.
I think I ran into issues with that.
Goal is to have a shared library libmemtest.so that can be put into LD_PRELOAD. That same binary needs to be dynamically linked by the Python library to grab the statistics out.
If we put the Pyo3 bindings into libmemtest.so, I was finding it caused some issues when used in LD_PRELOAD. The alternative would be to create a second shared library that dynamically links to libmemtest.so via Rust. But I felt it was just easier in the end to use ctypes here, since the API is so small.
| @@ -0,0 +1,37 @@ | |||
| """CLI for lance-memtest.""" | |||
There was a problem hiding this comment.
How is this CLI intended to be used? It seems more like a helper tool for debugging the library?
There was a problem hiding this comment.
Removed the stats command, as that's useless. It's mainly meant to get the library path so you can run:
LD_PRELOAD=$(lance-memtest) ...| ``` | ||
|
|
||
| The `io_mem_benchmark` fixture: | ||
| - Runs an optional warmup iteration (not measured) |
There was a problem hiding this comment.
Doesn't pytest benchmark already do warmups?
There was a problem hiding this comment.
Ah, I see, this is an alternative to pytest-benchmark, it doesn't use it under the hood.
| The `io_mem_benchmark` fixture: | ||
| - Runs an optional warmup iteration (not measured) | ||
| - Tracks IO stats via `dataset.io_stats_incremental()` | ||
| - Optionally tracks memory via `lance-memtest` if preloaded |
There was a problem hiding this comment.
Does it skip the test if lance-memtest is not preloaded?
There was a problem hiding this comment.
No, it just does the IO part. Output looks like this:
=========================================================== IO/Memory Benchmark Statistics ==================
Test Read IOPS Read Bytes Write IOPS Write Bytes
-----------------------------------------------------------------------------------------------------------
test_io_mem_basic_btree_search[small_strings-none] 2 1.8 MB 0 0.0 B
test_io_mem_basic_btree_search[small_strings-not_equal] 2 1.8 MB 0 0.0 B
test_io_mem_basic_btree_search[integers-none] 1 781.2 KB 0 0.0 B
test_io_mem_basic_btree_search[integers-not_equal] 1 781.2 KB 0 0.0 B
test_io_mem_basic_btree_search[small_strings-equal] 0 0.0 B 0 0.0 B
test_io_mem_basic_btree_search[small_strings-small_range] 0 0.0 B 0 0.0 B
test_io_mem_basic_btree_search[small_strings-large_in] 0 0.0 B 0 0.0 B
test_io_mem_basic_btree_search[integers-equal] 0 0.0 B 0 0.0 B
test_io_mem_basic_btree_search[integers-small_range] 0 0.0 B 0 0.0 B
test_io_mem_basic_btree_search[integers-large_in] 0 0.0 B 0 0.0 B…t#5461) Adding new capabilities for testing in benchmarking: 1. Can make assertions in unit tests about memory use. 2. Can write CI benchmarks that track memory use and IO statistics ## Testing memory use We add a new Python module called `memtest` which allows tracking memory statistics during particular sections of Python code. It works by using the `LD_PRELOAD` trick to interpose all calls to the glibc allocation APIs, and thus it captures all allocations that happen, even those from Python or other native Python extensions (such as `numpy` and `pyarrow`). To use it, you first need to run: ```shell export LD_PRELOAD=$(lance-memtest) ``` Then you can write assertions in tests like this: ```python with memtest.track() as get_stats: ds = lance.write_dataset( reader, tmp_path / "test.lance", ) stats = get_stats() assert stats["peak_bytes"] >= 5 * 1024 * 1024 assert stats["peak_bytes"] < 30 * 1024 * 1024 ``` ## Benchmarking memory use To use this with benchmarks, we introduce a custom pytest plugin that's similar to `pytest-benchmark` but tracks IO and memory statistics instead. ```python @pytest.mark.io_memory_benchmark() def test_io_mem_bencharm(io_mem_benchmark): ds = setup() def bench(ds): ds.to_table() io_mem_benchmark(ds) ``` This outputs a JSON report that is compatible with Bencher.dev's format and thus can be uploaded to the continuous benchmarking platform.
Adding new capabilities for testing in benchmarking:
Testing memory use
We add a new Python module called
memtestwhich allows tracking memory statistics during particular sections of Python code. It works by using theLD_PRELOADtrick to interpose all calls to the glibc allocation APIs, and thus it captures all allocations that happen, even those from Python or other native Python extensions (such asnumpyandpyarrow).To use it, you first need to run:
Then you can write assertions in tests like this:
Benchmarking memory use
To use this with benchmarks, we introduce a custom pytest plugin that's similar to
pytest-benchmarkbut tracks IO and memory statistics instead.This outputs a JSON report that is compatible with Bencher.dev's format and thus can be uploaded to the continuous benchmarking platform.