Add single instance for retrieving a buffer label

include/buffer_manager.h

@@ -278,9 +278,13 @@ namespace detail {
 			m_buffer_infos.at(bid).debug_name = debug_name;
 		}
 
-		std::string get_debug_name(const buffer_id bid) const {
-			std::lock_guard lock(m_mutex);
-			return m_buffer_infos.at(bid).debug_name;
+		std::string get_debug_label(const buffer_id bid) const {
+			std::string name;
+			{
+				std::lock_guard lock(m_mutex);
+				name = m_buffer_infos.at(bid).debug_name;
+			}
+			return !name.empty() ? fmt::format("B{} \"{}\"", bid, name) : fmt::format("B{}", bid);
 		}
 
 	  private:

include/debug.h

@@ -15,8 +15,6 @@ namespace debug {
 		return detail::get_buffer_name(buff);
 	}
 
-	inline void set_task_name(celerity::handler& cgh, const std::string& debug_name) {
-		detail::set_task_name(cgh, debug_name);
-	}
+	inline void set_task_name(celerity::handler& cgh, const std::string& debug_name) { detail::set_task_name(cgh, debug_name); }
 } // namespace debug
 } // namespace celerity

include/handler.h

@@ -376,7 +376,7 @@ class handler {
 	friend void detail::add_reduction(handler& cgh, const detail::reduction_info& rinfo);
 	friend void detail::extend_lifetime(handler& cgh, std::shared_ptr<detail::lifetime_extending_state> state);
 
-	friend void detail::set_task_name(handler &cgh, const std::string& debug_name);
+	friend void detail::set_task_name(handler& cgh, const std::string& debug_name);
 
 	detail::task_id m_tid;
 	detail::buffer_access_map m_access_map;
@@ -462,11 +462,9 @@ class handler {
 		}
 		// Note that cgf_diagnostics has a similar check, but we don't catch void side effects there.
 		if(!m_side_effects.empty()) { throw std::runtime_error{"Side effects cannot be used in device kernels"}; }
-		m_task =
-		    detail::task::make_device_compute(m_tid, geometry, std::move(launcher), std::move(m_access_map), std::move(m_reductions));
+		m_task = detail::task::make_device_compute(m_tid, geometry, std::move(launcher), std::move(m_access_map), std::move(m_reductions));
 
 		m_task->set_debug_name(m_usr_def_task_name.value_or(debug_name));
-
 	}
 
 	void create_collective_task(detail::collective_group_id cgid, std::unique_ptr<detail::command_launcher_storage_base> launcher) {
@@ -588,9 +586,7 @@ namespace detail {
 
 	inline void extend_lifetime(handler& cgh, std::shared_ptr<detail::lifetime_extending_state> state) { cgh.extend_lifetime(std::move(state)); }
 
-	inline void set_task_name(handler& cgh, const std::string& debug_name) {
-		cgh.m_usr_def_task_name = {debug_name};
-	}
+	inline void set_task_name(handler& cgh, const std::string& debug_name) { cgh.m_usr_def_task_name = {debug_name}; }
 
 	// TODO: The _impl functions in detail only exist during the grace period for deprecated reductions on const buffers; move outside again afterwards.
 	template <typename DataT, int Dims, typename BinaryOperation>

src/print_graph.cc

@@ -34,25 +34,17 @@ const char* task_type_string(const task_type tt) {
 	}
 }
 
-std::string get_buffer_label(const buffer_id bid, const std::string& name = "") {
-	// if there is no name defined, the name will be the buffer id.
-	// if there is a name we want "id name"
-	return !name.empty() ? fmt::format("B{} \"{}\"", bid, name) : fmt::format("B{}", bid);
-}
-
 void format_requirements(std::string& label, const reduction_list& reductions, const access_list& accesses, const side_effect_map& side_effects,
     const access_mode reduction_init_mode) {
 	for(const auto& [rid, bid, buffer_name, init_from_buffer] : reductions) {
 		auto rmode = init_from_buffer ? reduction_init_mode : cl::sycl::access::mode::discard_write;
 		const region scalar_region(box<3>({0, 0, 0}, {1, 1, 1}));
-		const std::string bl = get_buffer_label(bid, buffer_name);
-		fmt::format_to(std::back_inserter(label), "<br/>(R{}) <i>{}</i> {} {}", rid, detail::access::mode_traits::name(rmode), bl, scalar_region);
+		fmt::format_to(std::back_inserter(label), "<br/>(R{}) <i>{}</i> {} {}", rid, detail::access::mode_traits::name(rmode), buffer_name, scalar_region);
 	}
 
 	for(const auto& [bid, buffer_name, mode, req] : accesses) {
-		const std::string bl = get_buffer_label(bid, buffer_name);
 		// While uncommon, we do support chunks that don't require access to a particular buffer at all.
-		if(!req.empty()) { fmt::format_to(std::back_inserter(label), "<br/><i>{}</i> {} {}", detail::access::mode_traits::name(mode), bl, req); }
+		if(!req.empty()) { fmt::format_to(std::back_inserter(label), "<br/><i>{}</i> {} {}", detail::access::mode_traits::name(mode), buffer_name, req); }
 	}
 
 	for(const auto& [hoid, order] : side_effects) {
@@ -102,7 +94,7 @@ std::string get_command_label(const node_id local_nid, const command_record& cmd
 	auto add_reduction_id_if_reduction = [&]() {
 		if(cmd.reduction_id.has_value() && cmd.reduction_id != 0) { fmt::format_to(std::back_inserter(label), "(R{}) ", cmd.reduction_id.value()); }
 	};
-	const std::string buffer_label = cmd.buffer_id.has_value() ? get_buffer_label(cmd.buffer_id.value(), cmd.buffer_name) : "";
+	const std::string buffer_label = cmd.buffer_id.has_value() ? cmd.buffer_name : "";
 
 	switch(cmd.type) {
 	case command_type::epoch: {

src/recorders.cc

@@ -9,7 +9,7 @@ namespace celerity::detail {
 // Naming
 
 std::string get_buffer_name(const buffer_id bid, const buffer_manager* buff_man) {
-	return buff_man != nullptr ? buff_man->get_buffer_info(bid).debug_name : "";
+	return buff_man != nullptr ? buff_man->get_debug_label(bid) : fmt::format("B{}", bid);
 }
 
 // Tasks
@@ -91,7 +91,7 @@ std::optional<buffer_id> get_buffer_id(const abstract_command& cmd) {
 }
 
 std::string get_cmd_buffer_name(const std::optional<buffer_id>& bid, const buffer_manager* buff_mngr) {
-	if(buff_mngr == nullptr || !bid.has_value()) return "";
+	if(!bid.has_value()) return "";
 	return get_buffer_name(bid.value(), buff_mngr);
 }
 

src/worker_job.cc

@@ -202,10 +202,9 @@ namespace detail {
 					const auto acc_sr = access_map.get_requirements_for_nth_access(i, tsk->get_dimensions(), data.sr, tsk->get_global_size()).get_subrange();
 					const auto oob_sr = subrange<3>(oob_min, range_cast<3>(oob_max - oob_min));
 					const auto buffer_id = access_map.get_nth_access(i).first;
-					const auto buffer_name = m_buffer_mngr.get_debug_name(buffer_id);
 					CELERITY_ERROR("Out-of-bounds access in host task detected: Accessor {} for buffer {} attempted to access indices between {} which are "
 					               "outside of mapped subrange {}",
-					    i, (buffer_name.empty() ? fmt::format("{}", buffer_id) : buffer_name), oob_sr, acc_sr);
+					    i, m_buffer_mngr.get_debug_label(buffer_id), oob_sr, acc_sr);
 				}
 			}
 #endif
@@ -301,10 +300,9 @@ namespace detail {
 					const auto acc_sr = access_map.get_requirements_for_nth_access(i, tsk->get_dimensions(), data.sr, tsk->get_global_size()).get_subrange();
 					const auto oob_sr = subrange<3>(oob_min, range_cast<3>(oob_max - oob_min));
 					const auto buffer_id = access_map.get_nth_access(i).first;
-					const auto buffer_name = m_buffer_mngr.get_debug_name(buffer_id);
 					CELERITY_ERROR("Out-of-bounds access in kernel '{}' detected: Accessor {} for buffer {} attempted to access indices between {} which are "
 					               "outside of mapped subrange {}",
-					    tsk->get_debug_name(), i, (buffer_name.empty() ? fmt::format("{}", buffer_id) : buffer_name), oob_sr, acc_sr);
+					    tsk->get_debug_name(), i, m_buffer_mngr.get_debug_label(buffer_id), oob_sr, acc_sr);
 				}
 				sycl::free(m_oob_indices_per_accessor[i], m_queue.get_sycl_queue());
 			}

test/accessor_tests.cc

@@ -689,15 +689,15 @@ namespace detail {
 		const auto attempted_sr =
 		    subrange<3>{id_cast<3>(oob_idx_lo), range_cast<3>(oob_idx_hi - oob_idx_lo + id_cast<Dims>(range<Dims>(ones))) - range_cast<3>(range<Dims>(zeros))};
 		const auto unnamed_error_message =
-		    fmt::format("Out-of-bounds access in kernel 'celerity::detail::acc_out_of_bounds_kernel<{}>' detected: Accessor 0 for buffer 0 attempted to "
+		    fmt::format("Out-of-bounds access in kernel 'celerity::detail::acc_out_of_bounds_kernel<{}>' detected: Accessor 0 for buffer B0 attempted to "
 		                "access indices between {} which are outside of mapped subrange {}",
 		        Dims, attempted_sr, subrange_cast<3>(accessible_sr));
 		CHECK_THAT(lc->get_log(), Catch::Matchers::ContainsSubstring(unnamed_error_message));
 
-		const auto named_error_message =
-		    fmt::format("Out-of-bounds access in kernel 'celerity::detail::acc_out_of_bounds_kernel<{}>' detected: Accessor 1 for buffer {} attempted to "
-		                "access indices between {} which are outside of mapped subrange {}",
-		        Dims, buffer_name, attempted_sr, subrange_cast<3>(accessible_sr));
+		const auto named_error_message = fmt::format(
+		    "Out-of-bounds access in kernel 'celerity::detail::acc_out_of_bounds_kernel<{}>' detected: Accessor 1 for buffer B1 \"{}\" attempted to "
+		    "access indices between {} which are outside of mapped subrange {}",
+		    Dims, buffer_name, attempted_sr, subrange_cast<3>(accessible_sr));
 		CHECK_THAT(lc->get_log(), Catch::Matchers::ContainsSubstring(named_error_message));
 	}
 
@@ -740,12 +740,12 @@ namespace detail {
 
 		const auto attempted_sr =
 		    subrange<3>{id_cast<3>(oob_idx_lo), range_cast<3>(oob_idx_hi - oob_idx_lo + id_cast<Dims>(range<Dims>(ones))) - range_cast<3>(range<Dims>(zeros))};
-		const auto unnamed_error_message = fmt::format("Out-of-bounds access in host task detected: Accessor 0 for buffer 0 attempted to "
+		const auto unnamed_error_message = fmt::format("Out-of-bounds access in host task detected: Accessor 0 for buffer B0 attempted to "
 		                                               "access indices between {} which are outside of mapped subrange {}",
 		    attempted_sr, subrange_cast<3>(accessible_sr));
 		CHECK_THAT(lc->get_log(), Catch::Matchers::ContainsSubstring(unnamed_error_message));
 
-		const auto named_error_message = fmt::format("Out-of-bounds access in host task detected: Accessor 1 for buffer {} attempted to "
+		const auto named_error_message = fmt::format("Out-of-bounds access in host task detected: Accessor 1 for buffer B1 \"{}\" attempted to "
 		                                             "access indices between {} which are outside of mapped subrange {}",
 		    buffer_name, attempted_sr, subrange_cast<3>(accessible_sr));
 		CHECK_THAT(lc->get_log(), Catch::Matchers::ContainsSubstring(named_error_message));

test/debug_naming_tests.cc

@@ -19,9 +19,7 @@ TEST_CASE("debug names can be set and retrieved from tasks", "[debug]") {
 	auto tt = test_utils::task_test_context{};
 
 	SECTION("Host Task") {
-		const auto tid_a = test_utils::add_host_task(tt.tm, on_master_node, [&](handler& cgh) {
-			celerity::debug::set_task_name(cgh, task_name);
-		});
+		const auto tid_a = test_utils::add_host_task(tt.tm, on_master_node, [&](handler& cgh) { celerity::debug::set_task_name(cgh, task_name); });
 
 		const auto tid_b = test_utils::add_host_task(tt.tm, on_master_node, [&](handler& cgh) {});
 
@@ -30,9 +28,7 @@ TEST_CASE("debug names can be set and retrieved from tasks", "[debug]") {
 	}
 
 	SECTION("Compute Task") {
-		const auto tid_a = test_utils::add_compute_task<class compute_task>(tt.tm, [&](handler& cgh) {
-			celerity::debug::set_task_name(cgh, task_name);
-		});
+		const auto tid_a = test_utils::add_compute_task<class compute_task>(tt.tm, [&](handler& cgh) { celerity::debug::set_task_name(cgh, task_name); });
 
 		const auto tid_b = test_utils::add_compute_task<class compute_task_unnamed>(tt.tm, [&](handler& cgh) {});
 
@@ -41,9 +37,8 @@ TEST_CASE("debug names can be set and retrieved from tasks", "[debug]") {
 	}
 
 	SECTION("ND Range Task") {
-		const auto tid_a = test_utils::add_nd_range_compute_task<class nd_range_task>(tt.tm, [&](handler& cgh) {
-			celerity::debug::set_task_name(cgh, task_name);
-		});
+		const auto tid_a =
+		    test_utils::add_nd_range_compute_task<class nd_range_task>(tt.tm, [&](handler& cgh) { celerity::debug::set_task_name(cgh, task_name); });
 
 		const auto tid_b = test_utils::add_compute_task<class nd_range_task_unnamed>(tt.tm, [&](handler& cgh) {});
 
@@ -55,13 +50,14 @@ TEST_CASE("debug names can be set and retrieved from tasks", "[debug]") {
 TEST_CASE_METHOD(test_utils::runtime_fixture, "buffer_manager allows to set buffer debug names on  buffers", "[buffer_manager]") {
 	celerity::buffer<int, 1> buff_a(16);
 	std::string buff_name{"my_buffer"};
+	std::string buffer_label{fmt::format("B{} \"{}\"", detail::get_buffer_id(buff_a), buff_name)};
 	celerity::detail::runtime::get_instance().get_buffer_manager().set_debug_name(celerity::detail::get_buffer_id(buff_a), buff_name);
-	CHECK(celerity::detail::runtime::get_instance().get_buffer_manager().get_debug_name(celerity::detail::get_buffer_id(buff_a)) == buff_name);
+	CHECK(detail::runtime::get_instance().get_buffer_manager().get_debug_label(detail::get_buffer_id(buff_a)) == buffer_label);
 }
 
 
 namespace foo {
-	class MySecondKernel;
+class MySecondKernel;
 }
 
 template <typename T>