[RM] psalz feedback (partial)

include/scheduler.h

@@ -77,10 +77,12 @@ class scheduler {
 
 	std::unique_ptr<scheduler_detail::scheduler_impl> m_impl;
 
-	// accessible from scheduler_testspy
+	// used by test_benchmark_scheduler
 	scheduler(test_start_idle_tag, size_t num_nodes, node_id local_node_id, const system_info& system_info, scheduler::delegate* delegate,
 	    command_recorder* crec, instruction_recorder* irec, const policy_set& policy = {});
 	void test_invoke_thread_main();
+
+	// used by scheduler_testspy
 	void test_inspect(scheduler_detail::test_inspector inspector);
 };
 

src/instruction_graph_generator.cc

@@ -327,9 +327,10 @@ struct buffer_memory_state {
 	// Updated by both anticipate() and allocate_contiguously() to elide buffer resize allocations when possible.
 	box_vector<3> future_allocated_contiguous_boxes;
 
-	// Track the total number of allocations made during the buffer's lifetime to heuristically detect frequent re-sizes and split allocations that could have
-	// been merged with proper scheduler lookahead.
+	// Track the total number of allocations made during the buffer's lifetime to heuristically detect and warn about frequent re-sizes and split allocations
+	// that could have been merged with proper scheduler lookahead.
 	size_t total_allocations_performed = 0;
+	bool frequent_allocations_warning_emitted = false;
 
 	const buffer_allocation_state& get_allocation(const allocation_id aid) const {
 		const auto it = std::find_if(allocations.begin(), allocations.end(), [=](const buffer_allocation_state& a) { return a.aid == aid; });
@@ -606,9 +607,6 @@ class generator_impl {
 	/// emitting the fence, and buffer host-initialization user allocations after the buffer has been destroyed.
 	std::vector<allocation_id> m_unreferenced_user_allocations;
 
-	/// The number of allocations per buffer are tracked in buffer_memory_state, but we only want to emit the corresponding warning once.
-	bool m_frequent_allocation_warning_emitted = false;
-
 	/// True if a recorder is present and create() will call the `record_with` lambda passed as its last parameter.
 	bool is_recording() const { return m_recorder != nullptr; }
 
@@ -1084,13 +1082,14 @@ void generator_impl::allocate_contiguously(batch& current_batch, const buffer_id
 	memory.allocations.insert(memory.allocations.end(), std::make_move_iterator(new_allocations.begin()), std::make_move_iterator(new_allocations.end()));
 
 	// Heuristically detect excessive resizes, which should all be elided by scheduler lookahead (and iggen::anticipate()) in a well-behaved program.
-	if(!m_frequent_allocation_warning_emitted && memory.total_allocations_performed >= 10) {
+	if(!memory.frequent_allocations_warning_emitted && memory.total_allocations_performed >= 10) {
 		// This warning also covers all cases of the buffer_memory_state::allocations vector growing out of control, which quickly degrades scheduler
 		// performance in the current quadratic / cubic implementation of merge_overlapping_bounding_boxes / merge_connected_boxes.
-		CELERITY_WARN("Your program triggers frequent buffer allocations or resizes, which may degrade performance. If possible, avoid "
+		CELERITY_WARN("Your program triggers frequent allocations or resizes for buffer {}, which may degrade performance. If possible, avoid "
 		              "celerity::queue::fence(), celerity::queue::wait() and celerity::experimental::flush() between command groups of growing access "
-		              "patterns, or try increasing scheduler lookahead via celerity::experimental::set_lookahead().");
-		m_frequent_allocation_warning_emitted = true;
+		              "patterns, or try increasing scheduler lookahead via celerity::experimental::set_lookahead().",
+		    print_buffer_debug_label(bid));
+		memory.frequent_allocations_warning_emitted = true;
 	}
 }
 

src/scheduler.cc

@@ -110,7 +110,7 @@ class command_queue {
 		if(m_queue.empty()) return false;
 		if(lookahead == experimental::lookahead::none) return true;                        // unconditionally dequeue, and do not inspect scheduling hints
 		if(m_num_flushes_in_queue > 0) return true;                                        // force-dequeue until all flushing events are processed
-		if(!std::holds_alternative<event_command_available>(m_queue.front())) return true; // only command_events carry a hint and are thus worth delaying
+		if(!std::holds_alternative<event_command_available>(m_queue.front())) return true; // only commands carry a hint and are thus worth delaying
 		const auto& avail = std::get<event_command_available>(m_queue.front());
 		assert(avail.hint.has_value()); // only nullopt when lookahead == none, which we checked above
 		if(avail.hint == instruction_graph_generator::scheduling_hint::is_self_contained) return true; // don't delay scheduling of self-contained commands

test/scheduler_tests.cc

@@ -93,10 +93,10 @@ TEST_CASE("scheduler(lookahead::none) does not attempt to elide allocations", "[
 		CHECK(iq.count<device_kernel_instruction_record>() == num_timesteps);
 	});
 
-	CHECK(test_utils::log_contains_exact(
-	    log_level::warn, "Your program triggers frequent buffer allocations or resizes, which may degrade performance. If possible, avoid "
-	                     "celerity::queue::fence(), celerity::queue::wait() and celerity::experimental::flush() between command groups of growing access "
-	                     "patterns, or try increasing scheduler lookahead via celerity::experimental::set_lookahead()."));
+	CHECK(test_utils::log_contains_exact(log_level::warn,
+	    "Your program triggers frequent buffer allocations or resizes, which may degrade performance. If possible, avoid "
+	    "celerity::queue::fence(), celerity::queue::wait() and celerity::experimental::flush() between command groups of growing access "
+	    "patterns, or try increasing scheduler lookahead via celerity::experimental::set_lookahead()."));
 }
 
 TEST_CASE("scheduler(lookahead::infinite) does not flush commands unless forced to", "[scheduler][lookahead]") {