From d9d9ca80f8d315bc9d1f749c5971757973ce0b9b Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 20 Jun 2022 11:37:38 +0100 Subject: [PATCH 1/7] Increase benchmark cube size to reduce memory noise. --- benchmarks/benchmarks/experimental/ugrid/regions_combine.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/benchmarks/experimental/ugrid/regions_combine.py b/benchmarks/benchmarks/experimental/ugrid/regions_combine.py index 0cac84d0a8..5f77c640b3 100644 --- a/benchmarks/benchmarks/experimental/ugrid/regions_combine.py +++ b/benchmarks/benchmarks/experimental/ugrid/regions_combine.py @@ -31,7 +31,7 @@ class MixinCombineRegions: # Characterise time taken + memory-allocated, for various stages of combine # operations on cubesphere-like test data. - params = [4, 500] + params = [4, 2000] param_names = ["cubesphere-N"] # For use on 'track_addedmem_..' type benchmarks - result is too noisy. no_small_params = params[1:] From f1dd87ad31f00b9d76a8d7eae328ad10b1fa3e39 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 20 Jun 2022 12:05:05 +0100 Subject: [PATCH 2/7] Revert "Increase benchmark cube size to reduce memory noise." This reverts commit d9d9ca80f8d315bc9d1f749c5971757973ce0b9b. --- benchmarks/benchmarks/experimental/ugrid/regions_combine.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/benchmarks/experimental/ugrid/regions_combine.py b/benchmarks/benchmarks/experimental/ugrid/regions_combine.py index 5f77c640b3..0cac84d0a8 100644 --- a/benchmarks/benchmarks/experimental/ugrid/regions_combine.py +++ b/benchmarks/benchmarks/experimental/ugrid/regions_combine.py @@ -31,7 +31,7 @@ class MixinCombineRegions: # Characterise time taken + memory-allocated, for various stages of combine # operations on cubesphere-like test data. - params = [4, 2000] + params = [4, 500] param_names = ["cubesphere-N"] # For use on 'track_addedmem_..' type benchmarks - result is too noisy. no_small_params = params[1:] From 877dc4fbc14fa0cbf88c6aa6cfb8d26470111f08 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 20 Jun 2022 12:09:41 +0100 Subject: [PATCH 3/7] Memory benchmark results return minimum 5Mb. --- benchmarks/benchmarks/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/benchmarks/benchmarks/__init__.py b/benchmarks/benchmarks/__init__.py index 765eb2195d..a34c63a32f 100644 --- a/benchmarks/benchmarks/__init__.py +++ b/benchmarks/benchmarks/__init__.py @@ -69,7 +69,11 @@ def __exit__(self, *_): def addedmem_mb(self): """Return measured memory growth, in Mb.""" - return self.mb_after - self.mb_before + result = self.mb_after - self.mb_before + # Results <5Mb are too vulnerable to noise being interpreted as a true + # regression. + result = max(5.0, result) + return result @staticmethod def decorator(changed_params: list = None): From 4990584568704c2cd29dbf273e8a7988bdf8d345 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 20 Jun 2022 12:51:15 +0100 Subject: [PATCH 4/7] Simplify memory benchmark architecture. --- benchmarks/benchmarks/__init__.py | 46 ++++++------------- benchmarks/benchmarks/cperf/save.py | 2 +- .../experimental/ugrid/regions_combine.py | 10 ++-- benchmarks/benchmarks/save.py | 4 +- .../benchmarks/sperf/combine_regions.py | 8 ++-- benchmarks/benchmarks/sperf/save.py | 2 +- 6 files changed, 24 insertions(+), 48 deletions(-) diff --git a/benchmarks/benchmarks/__init__.py b/benchmarks/benchmarks/__init__.py index a34c63a32f..edc33d5cdc 100644 --- a/benchmarks/benchmarks/__init__.py +++ b/benchmarks/benchmarks/__init__.py @@ -4,7 +4,6 @@ # See COPYING and COPYING.LESSER in the root of the repository for full # licensing details. """Common code for benchmarks.""" -from functools import wraps from os import environ import resource @@ -76,45 +75,26 @@ def addedmem_mb(self): return result @staticmethod - def decorator(changed_params: list = None): + def decorator(decorated_func): """ Decorates this benchmark to track growth in resident memory during execution. Intended for use on ASV ``track_`` benchmarks. Applies the :class:`TrackAddedMemoryAllocation` context manager to the benchmark - code, sets the benchmark ``unit`` attribute to ``Mb``. Optionally - replaces the benchmark ``params`` attribute with ``changed_params`` - - useful to avoid testing very small memory volumes, where the results - are vulnerable to noise. - - Parameters - ---------- - changed_params : list - Replace the benchmark's ``params`` attribute with this list. + code, sets the benchmark ``unit`` attribute to ``Mb``. """ - if changed_params: - # Must make a copy for re-use safety! - _changed_params = list(changed_params) - else: - _changed_params = None - - def _inner_decorator(decorated_func): - @wraps(decorated_func) - def _inner_func(*args, **kwargs): - assert decorated_func.__name__[:6] == "track_" - # Run the decorated benchmark within the added memory context manager. - with TrackAddedMemoryAllocation() as mb: - decorated_func(*args, **kwargs) - return mb.addedmem_mb() - - if _changed_params: - # Replace the params if replacement provided. - _inner_func.params = _changed_params - _inner_func.unit = "Mb" - return _inner_func - - return _inner_decorator + + def _wrapper(*args, **kwargs): + assert decorated_func.__name__[:6] == "track_" + # Run the decorated benchmark within the added memory context + # manager. + with TrackAddedMemoryAllocation() as mb: + decorated_func(*args, **kwargs) + return mb.addedmem_mb() + + decorated_func.unit = "Mb" + return _wrapper def on_demand_benchmark(benchmark_object): diff --git a/benchmarks/benchmarks/cperf/save.py b/benchmarks/benchmarks/cperf/save.py index 63eb5c25fb..2eb60e2ab5 100644 --- a/benchmarks/benchmarks/cperf/save.py +++ b/benchmarks/benchmarks/cperf/save.py @@ -42,6 +42,6 @@ def _save_data(self, cube): def time_save_data_netcdf(self, data_type): self._save_data(self.cube) - @TrackAddedMemoryAllocation.decorator() + @TrackAddedMemoryAllocation.decorator def track_addedmem_save_data_netcdf(self, data_type): self._save_data(self.cube) diff --git a/benchmarks/benchmarks/experimental/ugrid/regions_combine.py b/benchmarks/benchmarks/experimental/ugrid/regions_combine.py index 0cac84d0a8..8ebf210416 100644 --- a/benchmarks/benchmarks/experimental/ugrid/regions_combine.py +++ b/benchmarks/benchmarks/experimental/ugrid/regions_combine.py @@ -33,8 +33,6 @@ class MixinCombineRegions: # operations on cubesphere-like test data. params = [4, 500] param_names = ["cubesphere-N"] - # For use on 'track_addedmem_..' type benchmarks - result is too noisy. - no_small_params = params[1:] def _parametrised_cache_filename(self, n_cubesphere, content_name): return f"cube_C{n_cubesphere}_{content_name}.nc" @@ -190,7 +188,7 @@ def setup(self, n_cubesphere): def time_create_combined_cube(self, n_cubesphere): self.recombine() - @TrackAddedMemoryAllocation.decorator(MixinCombineRegions.no_small_params) + @TrackAddedMemoryAllocation.decorator def track_addedmem_create_combined_cube(self, n_cubesphere): self.recombine() @@ -203,7 +201,7 @@ class CombineRegionsComputeRealData(MixinCombineRegions): def time_compute_data(self, n_cubesphere): _ = self.recombined_cube.data - @TrackAddedMemoryAllocation.decorator(MixinCombineRegions.no_small_params) + @TrackAddedMemoryAllocation.decorator def track_addedmem_compute_data(self, n_cubesphere): _ = self.recombined_cube.data @@ -220,7 +218,7 @@ def time_save(self, n_cubesphere): # Save to disk, which must compute data + stream it to file. save(self.recombined_cube, "tmp.nc") - @TrackAddedMemoryAllocation.decorator(MixinCombineRegions.no_small_params) + @TrackAddedMemoryAllocation.decorator def track_addedmem_save(self, n_cubesphere): save(self.recombined_cube, "tmp.nc") @@ -248,6 +246,6 @@ def time_stream_file2file(self, n_cubesphere): # Save to disk, which must compute data + stream it to file. save(self.recombined_cube, "tmp.nc") - @TrackAddedMemoryAllocation.decorator(MixinCombineRegions.no_small_params) + @TrackAddedMemoryAllocation.decorator def track_addedmem_stream_file2file(self, n_cubesphere): save(self.recombined_cube, "tmp.nc") diff --git a/benchmarks/benchmarks/save.py b/benchmarks/benchmarks/save.py index 730b63294d..bb1d2d8c82 100644 --- a/benchmarks/benchmarks/save.py +++ b/benchmarks/benchmarks/save.py @@ -24,8 +24,6 @@ class NetcdfSave: params = [[1, 600], [False, True]] param_names = ["cubesphere-N", "is_unstructured"] - # For use on 'track_addedmem_..' type benchmarks - result is too noisy. - no_small_params = [[600], [True]] def setup(self, n_cubesphere, is_unstructured): self.cube = make_cube_like_2d_cubesphere( @@ -50,7 +48,7 @@ def time_netcdf_save_mesh(self, n_cubesphere, is_unstructured): if is_unstructured: self._save_mesh(self.cube) - @TrackAddedMemoryAllocation.decorator(no_small_params) + @TrackAddedMemoryAllocation.decorator def track_addedmem_netcdf_save(self, n_cubesphere, is_unstructured): # Don't need to copy the cube here since track_ benchmarks don't # do repeats between self.setup() calls. diff --git a/benchmarks/benchmarks/sperf/combine_regions.py b/benchmarks/benchmarks/sperf/combine_regions.py index e68382c06e..d3d128c7d8 100644 --- a/benchmarks/benchmarks/sperf/combine_regions.py +++ b/benchmarks/benchmarks/sperf/combine_regions.py @@ -192,7 +192,7 @@ def setup( def time_create_combined_cube(self, n_cubesphere): self.recombine() - @TrackAddedMemoryAllocation.decorator() + @TrackAddedMemoryAllocation.decorator def track_addedmem_create_combined_cube(self, n_cubesphere): self.recombine() @@ -206,7 +206,7 @@ class ComputeRealData(Mixin): def time_compute_data(self, n_cubesphere): _ = self.recombined_cube.data - @TrackAddedMemoryAllocation.decorator() + @TrackAddedMemoryAllocation.decorator def track_addedmem_compute_data(self, n_cubesphere): _ = self.recombined_cube.data @@ -224,7 +224,7 @@ def time_save(self, n_cubesphere): # Save to disk, which must compute data + stream it to file. self.save_recombined_cube() - @TrackAddedMemoryAllocation.decorator() + @TrackAddedMemoryAllocation.decorator def track_addedmem_save(self, n_cubesphere): self.save_recombined_cube() @@ -252,6 +252,6 @@ def time_stream_file2file(self, n_cubesphere): # Save to disk, which must compute data + stream it to file. self.save_recombined_cube() - @TrackAddedMemoryAllocation.decorator() + @TrackAddedMemoryAllocation.decorator def track_addedmem_stream_file2file(self, n_cubesphere): self.save_recombined_cube() diff --git a/benchmarks/benchmarks/sperf/save.py b/benchmarks/benchmarks/sperf/save.py index 62c84a2619..dd33924c6c 100644 --- a/benchmarks/benchmarks/sperf/save.py +++ b/benchmarks/benchmarks/sperf/save.py @@ -41,7 +41,7 @@ def _save_mesh(self, cube): def time_save_cube(self, n_cubesphere, is_unstructured): self._save_cube(self.cube) - @TrackAddedMemoryAllocation.decorator() + @TrackAddedMemoryAllocation.decorator def track_addedmem_save_cube(self, n_cubesphere, is_unstructured): self._save_cube(self.cube) From decce4432b28f04f9d39fa8022f4f547312e0820 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 20 Jun 2022 15:03:44 +0100 Subject: [PATCH 5/7] Memory benchmarks RESULT_MINIMUM_MB attribute. --- benchmarks/benchmarks/__init__.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/benchmarks/benchmarks/__init__.py b/benchmarks/benchmarks/__init__.py index edc33d5cdc..227978c509 100644 --- a/benchmarks/benchmarks/__init__.py +++ b/benchmarks/benchmarks/__init__.py @@ -53,8 +53,22 @@ class TrackAddedMemoryAllocation: other_call() result = mb.addedmem_mb() + Attributes + ---------- + RESULT_MINIMUM_MB : float + The smallest result that should ever be returned, in + Mb. Results fluctuate from run to run (usually within + 1Mb) so if a result is sufficiently small this noise + will produce a before-after ratio over AVD's + detection threshold and be treated as 'signal. + Results smaller than this value will therefore be + returned as equal to this value, ensuring fractionally + small noise / no noise at all. + """ + RESULT_MINIMUM_MB = 5.0 + @staticmethod def process_resident_memory_mb(): return resource.getrusage(resource.RUSAGE_SELF).ru_maxrss / 1024.0 @@ -69,9 +83,8 @@ def __exit__(self, *_): def addedmem_mb(self): """Return measured memory growth, in Mb.""" result = self.mb_after - self.mb_before - # Results <5Mb are too vulnerable to noise being interpreted as a true - # regression. - result = max(5.0, result) + # Small results are too vulnerable to noise being interpreted as signal. + result = max(self.RESULT_MINIMUM_MB, result) return result @staticmethod From ef65aa03f1c3c8533e4f5482626bf1db9f5a20eb Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 20 Jun 2022 15:11:48 +0100 Subject: [PATCH 6/7] Typo. --- benchmarks/benchmarks/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/benchmarks/__init__.py b/benchmarks/benchmarks/__init__.py index 227978c509..1b206e1ebc 100644 --- a/benchmarks/benchmarks/__init__.py +++ b/benchmarks/benchmarks/__init__.py @@ -60,7 +60,7 @@ class TrackAddedMemoryAllocation: Mb. Results fluctuate from run to run (usually within 1Mb) so if a result is sufficiently small this noise will produce a before-after ratio over AVD's - detection threshold and be treated as 'signal. + detection threshold and be treated as 'signal'. Results smaller than this value will therefore be returned as equal to this value, ensuring fractionally small noise / no noise at all. From 0301a8091bb616f5bd4997e8798a9f3079d0aa9b Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Mon, 20 Jun 2022 15:46:22 +0100 Subject: [PATCH 7/7] Correct docstring indentation. --- benchmarks/benchmarks/__init__.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/benchmarks/benchmarks/__init__.py b/benchmarks/benchmarks/__init__.py index 1b206e1ebc..c86682ca4a 100644 --- a/benchmarks/benchmarks/__init__.py +++ b/benchmarks/benchmarks/__init__.py @@ -56,14 +56,12 @@ class TrackAddedMemoryAllocation: Attributes ---------- RESULT_MINIMUM_MB : float - The smallest result that should ever be returned, in - Mb. Results fluctuate from run to run (usually within - 1Mb) so if a result is sufficiently small this noise - will produce a before-after ratio over AVD's - detection threshold and be treated as 'signal'. - Results smaller than this value will therefore be - returned as equal to this value, ensuring fractionally - small noise / no noise at all. + The smallest result that should ever be returned, in Mb. Results + fluctuate from run to run (usually within 1Mb) so if a result is + sufficiently small this noise will produce a before-after ratio over + AVD's detection threshold and be treated as 'signal'. Results + smaller than this value will therefore be returned as equal to this + value, ensuring fractionally small noise / no noise at all. """