From 8b561971c4bad8e6d6c2e7a5a7a7e01dedb7055b Mon Sep 17 00:00:00 2001 From: Connor McEntee Date: Mon, 21 Apr 2025 17:01:55 -0600 Subject: [PATCH] feat: enable prof integration tests Refactor prof flag test changes and handle source inclusion. This enables prof tests for all integration tests. --- MODULE.bazel | 2 +- test/BUILD.bazel | 2 - test/integration/BUILD.bazel | 104 ++++++++++++++++++++++++++++++++--- test/unit/BUILD.bazel | 88 ++++++++++++++++++++--------- tools/test.bzl | 37 +++++++++++++ 5 files changed, 195 insertions(+), 38 deletions(-) create mode 100644 tools/test.bzl diff --git a/MODULE.bazel b/MODULE.bazel index fc27baff..e6458abd 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -1,6 +1,6 @@ module( name = "jemalloc", - version = "5.3.0-bcr.alpha.3", + version = "5.3.0-bcr.alpha.4", bazel_compatibility = [">=7.2.1"], # need support for "overlay" directory compatibility_level = 5, ) diff --git a/test/BUILD.bazel b/test/BUILD.bazel index 7ef07f23..3b6b04bc 100644 --- a/test/BUILD.bazel +++ b/test/BUILD.bazel @@ -71,8 +71,6 @@ cc_library( "include/test/test.h", "include/test/thd.h", "include/test/timer.h", - "//test/unit:arena_reset.c", # included by arena_reset_prof.c - "//test/unit:batch_alloc.c", # included by batch_alloc_prof.c ], copts = COPTS, includes = ["include"], diff --git a/test/integration/BUILD.bazel b/test/integration/BUILD.bazel index e9140e30..cac161de 100644 --- a/test/integration/BUILD.bazel +++ b/test/integration/BUILD.bazel @@ -1,5 +1,6 @@ load("@rules_cc//cc:defs.bzl", "cc_test") load("//tools:cc.bzl", "COPTS") +load("//tools:test.bzl", "join_conf", "test_name") load("//tools:transition.bzl", "PLATFORM", "transition_default_constraints") INTEGRATION_TESTS = [ @@ -32,15 +33,15 @@ INTEGRATION_TESTS = [ INTEGRATION_DECAY_TESTS = [ { - "name": "%s_%s" % ( - test["name"] if "name" in test else test["src"].removesuffix(".c").replace("/", "_"), + "name": test_name( + test, config["suffix"], ), "src": test["src"], - "conf": "%s,%s" % ( - test["conf"], + "conf": join_conf( + test.get("conf"), config["conf"], - ) if "conf" in test else config["conf"], + ), } for config in [ { @@ -55,6 +56,35 @@ INTEGRATION_DECAY_TESTS = [ for test in INTEGRATION_TESTS ] +INTEGRATION_PROF_TESTS = [ + { + "name": test_name( + test, + config["suffix"], + ), + "src": test["src"], + "conf": join_conf( + test.get("conf"), + config["conf"], + ), + "target_compatible_with": select({ + "//settings/flags:prof": [], + "//conditions:default": ["@platforms//:incompatible"], + }), + } + for config in [ + { + "suffix": "prof", + "conf": "prof:true", + }, + { + "suffix": "prof_active_false", + "conf": "prof:true,prof_active:false", + }, + ] + for test in INTEGRATION_TESTS +] + transition_default_constraints( name = "testlib", testonly = True, @@ -72,13 +102,14 @@ transition_default_constraints( ) [cc_test( - name = test["name"] if "name" in test else test["src"].removesuffix(".c").replace("/", "_"), + name = test_name(test), srcs = [test["src"]], copts = COPTS, env = {"JE_TEST_MALLOC_CONF": test.get("conf", "")}, local_defines = ["JEMALLOC_INTEGRATION_TEST"], + target_compatible_with = test.get("target_compatible_with", []), deps = [":testlib"], -) for test in INTEGRATION_TESTS + INTEGRATION_DECAY_TESTS] +) for test in INTEGRATION_TESTS + INTEGRATION_DECAY_TESTS + INTEGRATION_PROF_TESTS] INTEGRATION_CPP_TESTS = [ {"src": "cpp/basic.cpp"}, @@ -92,6 +123,60 @@ INTEGRATION_CPP_TESTS = [ }, ] +INTEGRATION_CPP_DECAY_TESTS = [ + { + "name": test_name( + test, + config["suffix"], + ), + "src": test["src"], + "conf": join_conf( + test.get("conf"), + config["conf"], + ), + } + for config in [ + { + "suffix": "decay_minus_one", + "conf": "dirty_decay_ms:-1,muzzy_decay_ms:-1", + }, + { + "suffix": "decay_zero", + "conf": "dirty_decay_ms:0,muzzy_decay_ms:0", + }, + ] + for test in INTEGRATION_CPP_TESTS +] + +INTEGRATION_CPP_PROF_TESTS = [ + { + "name": test_name( + test, + config["suffix"], + ), + "src": test["src"], + "conf": join_conf( + test.get("conf"), + config["conf"], + ), + "target_compatible_with": select({ + "//settings/flags:prof": [], + "//conditions:default": ["@platforms//:incompatible"], + }), + } + for config in [ + { + "suffix": "prof", + "conf": "prof:true", + }, + { + "suffix": "prof_active_false", + "conf": "prof:true,prof_active:false", + }, + ] + for test in INTEGRATION_CPP_TESTS +] + transition_default_constraints( name = "testlib_cpp", testonly = True, @@ -109,10 +194,11 @@ transition_default_constraints( ) [cc_test( - name = test["name"] if "name" in test else test["src"].removesuffix(".cpp").replace("/", "_"), + name = test_name(test), srcs = [test["src"]], copts = COPTS, env = {"JE_TEST_MALLOC_CONF": test.get("conf", "")}, local_defines = ["JEMALLOC_INTEGRATION_CPP_TEST"], + target_compatible_with = test.get("target_compatible_with", []), deps = [":testlib_cpp"], -) for test in INTEGRATION_CPP_TESTS] +) for test in INTEGRATION_CPP_TESTS + INTEGRATION_CPP_DECAY_TESTS + INTEGRATION_CPP_PROF_TESTS] diff --git a/test/unit/BUILD.bazel b/test/unit/BUILD.bazel index 6768ca24..67722b78 100644 --- a/test/unit/BUILD.bazel +++ b/test/unit/BUILD.bazel @@ -1,34 +1,19 @@ load("@rules_cc//cc:defs.bzl", "cc_test") load("//tools:cc.bzl", "COPTS") +load("//tools:test.bzl", "join_conf", "test_name") load("//tools:transition.bzl", "PLATFORM", "transition_default_constraints") -exports_files( - [ - "arena_reset.c", - "batch_alloc.c", - ], - visibility = ["//:__subpackages__"], -) - UNIT_TESTS = [ {"src": "a0.c"}, { "src": "arena_decay.c", "conf": "dirty_decay_ms:1000,muzzy_decay_ms:1000,tcache_max:1024", }, - { - "src": "arena_reset_prof.c", - "conf": "prof:true,lg_prof_sample:0", - }, {"src": "arena_reset.c"}, {"src": "atomic.c"}, {"src": "background_thread.c"}, {"src": "background_thread_enable.c"}, {"src": "base.c"}, - { - "src": "batch_alloc_prof.c", - "conf": "prof:true,lg_prof_sample:14", - }, { "src": "batch_alloc.c", "conf": "tcache_gc_incr_bytes:2147483648", @@ -75,13 +60,13 @@ UNIT_TESTS = [ "conf": "abort:false,zero:false,junk:true", }, { - "name": "unit_junk_alloc", - "src": "junk.c", + "src": "junk_alloc.c", + "deps": [":junk_c"], "conf": "abort:false,zero:false,junk:alloc", }, { - "name": "unit_junk_free", - "src": "junk.c", + "src": "junk_free.c", + "deps": [":junk_c"], "conf": "abort:false,zero:false,junk:free", }, {"src": "log.c"}, @@ -125,7 +110,7 @@ UNIT_TESTS = [ { "src": "prof_idump.c", "conf": "tcache:false", - "prof_conf": ",prof:true,prof_accum:true,prof_active:false,lg_prof_sample:0,lg_prof_interval:0", + "prof_conf": "prof:true,prof_accum:true,prof_active:false,lg_prof_sample:0,lg_prof_interval:0", }, { "src": "prof_log.c", @@ -233,17 +218,68 @@ transition_default_constraints( visibility = ["//:__subpackages__"], ) +# Handle C source inclusions +[ + cc_library( + name = src.replace(".", "_"), + testonly = True, + hdrs = [src], + tags = ["manual"], + visibility = ["//visibility:private"], + ) + for src in [ + "arena_reset.c", + "batch_alloc.c", + "junk.c", + ] +] + [cc_test( - name = test["name"] if "name" in test else test["src"].removesuffix(".c").replace("/", "_"), + name = test_name(test), srcs = [test["src"]], copts = COPTS, env = select({ - "//settings/flags:prof": { - "JE_TEST_MALLOC_CONF": test.get("conf", "") + test.get("prof_conf", ""), - }, + "//settings/flags:prof": {"JE_TEST_MALLOC_CONF": join_conf( + test.get("conf", ""), + test.get("prof_conf", ""), + )}, "//conditions:default": {"JE_TEST_MALLOC_CONF": test.get("conf", "")}, }), linkstatic = True, local_defines = ["JEMALLOC_UNIT_TEST"], - deps = [":testlib"], + deps = [":testlib"] + test.get("deps", []), ) for test in UNIT_TESTS] + +cc_test( + name = "arena_reset_prof", + srcs = ["arena_reset_prof.c"], + copts = COPTS, + env = {"JE_TEST_MALLOC_CONF": "prof:true,lg_prof_sample:0"}, + linkstatic = True, + local_defines = ["JEMALLOC_UNIT_TEST"], + target_compatible_with = select({ + "//settings/flags:prof": [], + "//conditions:default": ["@platforms//:incompatible"], + }), + deps = [ + ":arena_reset_c", + ":testlib", + ], +) + +cc_test( + name = "batch_alloc_prof", + srcs = ["batch_alloc_prof.c"], + copts = COPTS, + env = {"JE_TEST_MALLOC_CONF": "prof:true,lg_prof_sample:14"}, + linkstatic = True, + local_defines = ["JEMALLOC_UNIT_TEST"], + target_compatible_with = select({ + "//settings/flags:prof": [], + "//conditions:default": ["@platforms//:incompatible"], + }), + deps = [ + ":batch_alloc_c", + ":testlib", + ], +) diff --git a/tools/test.bzl b/tools/test.bzl new file mode 100644 index 00000000..1b17f2d6 --- /dev/null +++ b/tools/test.bzl @@ -0,0 +1,37 @@ +"Various test related utility functions" + +def join_conf(conf1, conf2): + """Joins two configuration strings with a ',' delimiter. + + Args: + conf1: First configuration string or None + conf2: Second configuration string or None + + Returns: + A properly joined configuration string without dangling commas. + """ + + if conf1 and conf2: + return "%s,%s" % (conf1, conf2) + elif not conf1: + return conf2 + elif not conf2: + return conf1 + else: + return "" + +def test_name(data, suffix = None): + """Generates a test name based on the source file name. + + Args: + data: Dict with test data containing 'src' and optional 'name' key + suffix: String suffix to remove from the source file name + + Returns: + A string representing the test name. + """ + name = data.get("name", data["src"].lower().removesuffix(".c").removesuffix(".cpp").replace("/", "_")) + if suffix: + return "%s_%s" % (name, suffix) + else: + return name