From 40d7d57d34face84fdea8c64ae1ff887e582ce27 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Wed, 23 Oct 2024 22:21:00 -0400 Subject: [PATCH 01/22] Always constrain the number of in-flight events --- docs/howto/other-howtos.md | 52 +++++++++--------- .../InteractiveStreamingExample.cc | 3 +- .../JANA/Engine/JArrowProcessingController.cc | 6 +- src/libraries/JANA/Topology/JEventPool.h | 55 ++++--------------- .../JANA/Topology/JTopologyBuilder.cc | 25 +++------ .../JANA/Topology/JTopologyBuilder.h | 4 +- .../unit_tests/Components/JFactoryTests.cc | 4 +- .../unit_tests/Components/UnfoldTests.cc | 8 +-- .../unit_tests/Engine/ArrowActivationTests.cc | 4 +- .../unit_tests/Engine/SchedulerTests.cc | 9 ++- .../unit_tests/Topology/ArrowTests.cc | 4 +- .../unit_tests/Topology/JPoolTests.cc | 45 +-------------- .../unit_tests/Topology/TopologyTests.cc | 16 ++---- 13 files changed, 68 insertions(+), 167 deletions(-) diff --git a/docs/howto/other-howtos.md b/docs/howto/other-howtos.md index bc08cc8d1..aaad3b7ae 100644 --- a/docs/howto/other-howtos.md +++ b/docs/howto/other-howtos.md @@ -91,22 +91,22 @@ The `JTest` plugin lets you test JANA's performance for different workloads. It | Name | Type | Default | Description | |:-----|:-----|:------------|:--------| -jtest:parser:cputime_ms | int | 0 | Time spent during parsing -jtest:parser:cputime_spread | int | 0.25 | Spread of time spent during parsing -jtest:parser:bytes | int | 2000000 | Bytes written during parsing -jtest:parser:bytes_spread | double | 0.25 | Spread of bytes written during parsing -jtest:disentangler:cputime_ms | int | 20 | Time spent during disentangling -jtest:disentangler:cputime_spread | double | 0.25 | Spread of time spent during disentangling -jtest:disentangler:bytes | int | 500000 | Bytes written during disentangling -jtest:disentangler:bytes_spread | double | 0.25 | Spread of bytes written during disentangling -jtest:tracker:cputime_ms | int | 200 | Time spent during tracking -jtest:tracker:cputime_spread | double | 0.25 | Spread of time spent during tracking -jtest:tracker:bytes | int | 1000 | Bytes written during tracking -jtest:tracker:bytes_spread | double | 0.25 | Spread of bytes written during tracking -jtest:plotter:cputime_ms | int | 0 | Time spent during plotting -jtest:plotter:cputime_spread | double | 0.25 | Spread of time spent during plotting -jtest:plotter:bytes | int | 1000 | Bytes written during plotting -jtest:plotter:bytes_spread | double | 0.25 | Spread of bytes written during plotting +| jtest:parser:cputime_ms | int | 0 | Time spent during parsing | +| jtest:parser:cputime_spread | int | 0.25 | Spread of time spent during parsing | +| jtest:parser:bytes | int | 2000000 | Bytes written during parsing | +| jtest:parser:bytes_spread | double | 0.25 | Spread of bytes written during parsing | +| jtest:disentangler:cputime_ms | int | 20 | Time spent during disentangling | +| jtest:disentangler:cputime_spread | double | 0.25 | Spread of time spent during disentangling | +| jtest:disentangler:bytes | int | 500000 | Bytes written during disentangling | +| jtest:disentangler:bytes_spread | double | 0.25 | Spread of bytes written during disentangling | +| jtest:tracker:cputime_ms | int | 200 | Time spent during tracking | +| jtest:tracker:cputime_spread | double | 0.25 | Spread of time spent during tracking | +| jtest:tracker:bytes | int | 1000 | Bytes written during tracking | +| jtest:tracker:bytes_spread | double | 0.25 | Spread of bytes written during tracking | +| jtest:plotter:cputime_ms | int | 0 | Time spent during plotting | +| jtest:plotter:cputime_spread | double | 0.25 | Spread of time spent during plotting | +| jtest:plotter:bytes | int | 1000 | Bytes written during plotting | +| jtest:plotter:bytes_spread | double | 0.25 | Spread of bytes written during plotting | @@ -114,23 +114,21 @@ The following parameters are used for benchmarking: | Name | Type | Default | Description | |:-----|:-----|:------------|:--------| -benchmark:nsamples | int | 15 | Number of measurements made for each thread count -benchmark:minthreads | int | 1 | Minimum thread count -benchmark:maxthreads | int | ncores | Maximum thread count -benchmark:threadstep | int | 1 | Thread count increment -benchmark:resultsdir | string | JANA_Test_Results | Directory name for benchmark test results +| benchmark:nsamples | int | 15 | Number of measurements made for each thread count | +| benchmark:minthreads | int | 1 | Minimum thread count | +| benchmark:maxthreads | int | ncores | Maximum thread count | +| benchmark:threadstep | int | 1 | Thread count increment | +| benchmark:resultsdir | string | JANA_Test_Results | Directory name for benchmark test results | The following parameters are more advanced, but may come in handy when doing performance tuning: | Name | Type | Default | Description | |:-----|:-----|:------------|:--------| -jana:event_pool_size | int | nthreads | The number of events which may be in-flight at once -jana:limit_total_events_in_flight | bool | 1 | Whether the number of in-flight events should be limited -jana:affinity | int | 0 | Thread pinning strategy. 0: None. 1: Minimize number of memory localities. 2: Minimize number of hyperthreads. -jana:locality | int | 0 | Memory locality strategy. 0: Global. 1: Socket-local. 2: Numa-domain-local. 3. Core-local. 4. Cpu-local -jana:enable_stealing | bool | 0 | Allow threads to pick up work from a different memory location if their local mailbox is empty. -jana:event_queue_threshold | int | 80 | Mailbox buffer size +| jana:max_inflight_events | int | nthreads | The number of events which may be in-flight at once. Should be at least `nthreads`, more gives better load balancing. | +| jana:affinity | int | 0 | Thread pinning strategy. 0: None. 1: Minimize number of memory localities. 2: Minimize number of hyperthreads. | +| jana:locality | int | 0 | Memory locality strategy. 0: Global. 1: Socket-local. 2: Numa-domain-local. 3. Core-local. 4. Cpu-local | +| jana:enable_stealing | bool | 0 | Allow threads to pick up work from a different memory location if their local mailbox is empty. | Creating code skeletons diff --git a/src/examples/InteractiveStreamingExample/InteractiveStreamingExample.cc b/src/examples/InteractiveStreamingExample/InteractiveStreamingExample.cc index 9ed2be6ed..ab6cfcf0b 100644 --- a/src/examples/InteractiveStreamingExample/InteractiveStreamingExample.cc +++ b/src/examples/InteractiveStreamingExample/InteractiveStreamingExample.cc @@ -77,8 +77,7 @@ void InitPlugin(JApplication* app) { app->SetParameterValue("nthreads", 4); app->SetParameterValue("jana:extended_report", true); - app->SetParameterValue("jana:limit_total_events_in_flight", true); - app->SetParameterValue("jana:event_pool_size", 16); + app->SetParameterValue("jana:max_inflight_events", 16); // TODO: Consider making streamDet:sub_socket be the 'source_name', and use JESG to switch between JSES and DecodeDASSource // TODO: Improve parametermanager interface diff --git a/src/libraries/JANA/Engine/JArrowProcessingController.cc b/src/libraries/JANA/Engine/JArrowProcessingController.cc index 5f717db7b..9ee0e2097 100644 --- a/src/libraries/JANA/Engine/JArrowProcessingController.cc +++ b/src/libraries/JANA/Engine/JArrowProcessingController.cc @@ -159,14 +159,10 @@ bool JArrowProcessingController::is_timed_out() { auto metrics = measure_performance(); int timeout_s; - if (metrics->total_uptime_s < (m_warmup_timeout_s * m_topology->m_pool_capacity * 1.0) / metrics->thread_count) { + if (metrics->total_uptime_s < (m_warmup_timeout_s * m_topology->m_max_inflight_events * 1.0) / metrics->thread_count) { // We are at the beginning and not all events have necessarily had a chance to warm up timeout_s = m_warmup_timeout_s; } - else if (!m_topology->m_limit_total_events_in_flight) { - // New events are constantly emitted, each of which may contain jfactorysets which need to be warmed up - timeout_s = m_warmup_timeout_s; - } else { timeout_s = m_timeout_s; } diff --git a/src/libraries/JANA/Topology/JEventPool.h b/src/libraries/JANA/Topology/JEventPool.h index 4ac26491b..7bf8fe774 100644 --- a/src/libraries/JANA/Topology/JEventPool.h +++ b/src/libraries/JANA/Topology/JEventPool.h @@ -24,7 +24,6 @@ class JEventPool { size_t m_pool_size; size_t m_location_count; - bool m_limit_total_events_in_flight; std::shared_ptr m_component_manager; JEventLevel m_level; @@ -34,17 +33,14 @@ class JEventPool { inline JEventPool(std::shared_ptr component_manager, size_t pool_size, size_t location_count, - bool limit_total_events_in_flight, JEventLevel level = JEventLevel::PhysicsEvent) : m_pool_size(pool_size) , m_location_count(location_count) - , m_limit_total_events_in_flight(limit_total_events_in_flight) , m_component_manager(component_manager) , m_level(level) { assert(m_location_count >= 1); - assert(m_pool_size > 0 || !m_limit_total_events_in_flight); m_pools = std::unique_ptr(new LocalPool[m_location_count]()); @@ -81,14 +77,7 @@ class JEventPool { std::lock_guard lock(pool.mutex); if (pool.available_items.empty()) { - if (m_limit_total_events_in_flight) { - return nullptr; - } - else { - auto t = new std::shared_ptr(); - configure_item(t); - return t; - } + return nullptr; } else { std::shared_ptr* item = pool.available_items.back(); @@ -112,15 +101,13 @@ class JEventPool { LocalPool& pool = m_pools[l % m_location_count]; // Check if item came from this location - if ((item >= &(pool.items[0])) && (item <= &(pool.items[m_pool_size-1]))) { + if (pool.available_items.size() < m_pool_size) { std::lock_guard lock(pool.mutex); pool.available_items.push_back(item); return; } - } - // Otherwise it was allocated on the heap - delete item; + throw JException("Event returned to already-full pool"); } @@ -133,38 +120,18 @@ class JEventPool { size_t available_count = pool.available_items.size(); - if (m_limit_total_events_in_flight && available_count < min_count) { + if (available_count < min_count) { // Exit immmediately if we can't reach the minimum return 0; } - if (m_limit_total_events_in_flight) { - // Return as many as we can. We aren't allowed to create any more - size_t count = std::min(available_count, max_count); - for (size_t i=0; i* t = pool.available_items.back(); - pool.available_items.pop_back(); - dest[i] = t; - } - return count; - } - else { - // Try to minimize number of allocations, as long as we meet min_count - size_t count = std::min(available_count, max_count); - size_t i=0; - for (i=0; i* t = pool.available_items.back(); - pool.available_items.pop_back(); - dest[i] = t; - } - for (; i; - configure_item(t); - dest[i] = t; - } - return i; + // Return as many as we can. We aren't allowed to create any more + size_t count = std::min(available_count, max_count); + for (size_t i=0; i* t = pool.available_items.back(); + pool.available_items.pop_back(); + dest[i] = t; } + return count; } void push(std::shared_ptr** source, size_t count, bool clear_event, size_t location) { diff --git a/src/libraries/JANA/Topology/JTopologyBuilder.cc b/src/libraries/JANA/Topology/JTopologyBuilder.cc index 0e4b53181..6c9cd790f 100644 --- a/src/libraries/JANA/Topology/JTopologyBuilder.cc +++ b/src/libraries/JANA/Topology/JTopologyBuilder.cc @@ -94,10 +94,7 @@ void JTopologyBuilder::create_topology() { mapping.initialize(static_cast(m_affinity), static_cast(m_locality)); - event_pool = new JEventPool(m_components, - m_pool_capacity, - m_location_count, - m_limit_total_events_in_flight); + event_pool = new JEventPool(m_components, m_max_inflight_events, m_location_count); if (m_configure_topology) { m_configure_topology(*this); @@ -127,22 +124,16 @@ void JTopologyBuilder::acquire_services(JServiceLocator *sl) { // We parse the 'nthreads' parameter two different ways for backwards compatibility. if (m_params->Exists("nthreads")) { if (m_params->GetParameterValue("nthreads") == "Ncores") { - m_pool_capacity = JCpuInfo::GetNumCpus(); + m_max_inflight_events = JCpuInfo::GetNumCpus(); } else { - m_pool_capacity = m_params->GetParameterValue("nthreads"); + m_max_inflight_events = m_params->GetParameterValue("nthreads"); } - m_queue_capacity = m_pool_capacity; } - m_params->SetDefaultParameter("jana:event_pool_size", m_pool_capacity, - "Sets the initial size of the event pool. Having too few events starves the workers; having too many consumes memory and introduces overhead from extra factory initializations") - ->SetIsAdvanced(true); - m_params->SetDefaultParameter("jana:limit_total_events_in_flight", m_limit_total_events_in_flight, - "Controls whether the event pool is allowed to automatically grow beyond jana:event_pool_size") - ->SetIsAdvanced(true); - m_params->SetDefaultParameter("jana:event_queue_threshold", m_queue_capacity, - "Max number of events allowed on the main event queue. Higher => Better load balancing; Lower => Fewer events in flight") + m_params->SetDefaultParameter("jana:max_inflight_events", m_max_inflight_events, + "The number of events which may be in-flight at once. Should be at least `nthreads` to prevent starvation; more gives better load balancing.") ->SetIsAdvanced(true); + m_params->SetDefaultParameter("jana:enable_stealing", m_enable_stealing, "Enable work stealing. Improves load balancing when jana:locality != 0; otherwise does nothing.") ->SetIsAdvanced(true); @@ -157,7 +148,7 @@ void JTopologyBuilder::acquire_services(JServiceLocator *sl) { void JTopologyBuilder::connect(JArrow* upstream, size_t up_index, JArrow* downstream, size_t down_index) { - auto queue = new EventQueue(m_queue_capacity, mapping.get_loc_count(), m_enable_stealing); + auto queue = new EventQueue(m_max_inflight_events, mapping.get_loc_count(), m_enable_stealing); queues.push_back(queue); size_t i = 0; @@ -260,7 +251,7 @@ void JTopologyBuilder::attach_level(JEventLevel current_level, JUnfoldArrow* par // -------------------------- // 0. Pool // -------------------------- - JEventPool* pool_at_level = new JEventPool(m_components, m_pool_capacity, m_location_count, m_limit_total_events_in_flight, current_level); + JEventPool* pool_at_level = new JEventPool(m_components, m_max_inflight_events, m_location_count, current_level); pools.push_back(pool_at_level); // Hand over ownership of the pool to the topology // -------------------------- diff --git a/src/libraries/JANA/Topology/JTopologyBuilder.h b/src/libraries/JANA/Topology/JTopologyBuilder.h index 7ee9996c3..b02a72d27 100644 --- a/src/libraries/JANA/Topology/JTopologyBuilder.h +++ b/src/libraries/JANA/Topology/JTopologyBuilder.h @@ -34,11 +34,9 @@ class JTopologyBuilder : public JService { std::vector pools; // Pools shared between arrows // Topology configuration - size_t m_pool_capacity = 4; - size_t m_queue_capacity = 4; + size_t m_max_inflight_events = 4; size_t m_location_count = 1; bool m_enable_stealing = false; - bool m_limit_total_events_in_flight = true; int m_affinity = 0; int m_locality = 0; diff --git a/src/programs/unit_tests/Components/JFactoryTests.cc b/src/programs/unit_tests/Components/JFactoryTests.cc index be2d681db..7e4c475d8 100644 --- a/src/programs/unit_tests/Components/JFactoryTests.cc +++ b/src/programs/unit_tests/Components/JFactoryTests.cc @@ -374,7 +374,7 @@ TEST_CASE("JFactory_CallbackSequence") { JApplication app; app.Add(new JFactoryGeneratorT()); app.SetParameterValue("autoactivate", "JFactoryTestDummyObject"); - app.SetParameterValue("jana:event_pool_size", 1); + app.SetParameterValue("jana:max_inflight_events", 1); SECTION("NoRunNumber") { app.Add(new JEventSource); @@ -429,7 +429,7 @@ TEST_CASE("JFactory_ExceptionHandling") { JApplication app; app.Add(new JFactoryGeneratorT()); app.SetParameterValue("autoactivate", "JFactoryTestDummyObject"); - app.SetParameterValue("jana:event_pool_size", 1); + app.SetParameterValue("jana:max_inflight_events", 1); SECTION("ExceptOnInit") { app.Add(new SourceWithRunNumberChange); diff --git a/src/programs/unit_tests/Components/UnfoldTests.cc b/src/programs/unit_tests/Components/UnfoldTests.cc index e7a2cf24c..3fa1341c9 100644 --- a/src/programs/unit_tests/Components/UnfoldTests.cc +++ b/src/programs/unit_tests/Components/UnfoldTests.cc @@ -46,8 +46,8 @@ TEST_CASE("UnfoldTests_Basic") { app.Initialize(); auto jcm = app.GetService(); - JEventPool parent_pool {jcm, 5, 1, true, JEventLevel::Timeslice}; // size=5, locations=1, limit_total_events_in_flight=true - JEventPool child_pool {jcm, 5, 1, true, JEventLevel::PhysicsEvent}; + JEventPool parent_pool {jcm, 5, 1, JEventLevel::Timeslice}; // size=5, locations=1, limit_total_events_in_flight=true + JEventPool child_pool {jcm, 5, 1, JEventLevel::PhysicsEvent}; JMailbox parent_queue {3}; // size JMailbox child_queue {3}; @@ -89,8 +89,8 @@ TEST_CASE("FoldArrowTests") { // We only use these to obtain preconfigured JEvents - JEventPool parent_pool {jcm, 5, 1, true, JEventLevel::Timeslice}; // size=5, locations=1, limit_total_events_in_flight=true - JEventPool child_pool {jcm, 5, 1, true, JEventLevel::PhysicsEvent}; + JEventPool parent_pool {jcm, 5, 1, JEventLevel::Timeslice}; // size=5, locations=1, limit_total_events_in_flight=true + JEventPool child_pool {jcm, 5, 1, JEventLevel::PhysicsEvent}; // We set up our test cases by putting events on these queues JMailbox*> child_in; diff --git a/src/programs/unit_tests/Engine/ArrowActivationTests.cc b/src/programs/unit_tests/Engine/ArrowActivationTests.cc index 11847d614..bb1d5679b 100644 --- a/src/programs/unit_tests/Engine/ArrowActivationTests.cc +++ b/src/programs/unit_tests/Engine/ArrowActivationTests.cc @@ -25,8 +25,8 @@ TEST_CASE("ArrowActivationTests") { auto q2 = new JMailbox(); auto q3 = new JMailbox(); - auto p1 = new JEventPool(jcm, 0,1,false); - auto p2 = new JEventPool(jcm, 0,1,false); + auto p1 = new JEventPool(jcm, 10,1); + auto p2 = new JEventPool(jcm, 10,1); auto emit_rand_ints = new RandIntArrow("emit_rand_ints", p1, q1); auto multiply_by_two = new MultByTwoArrow("multiply_by_two", q1, q2); diff --git a/src/programs/unit_tests/Engine/SchedulerTests.cc b/src/programs/unit_tests/Engine/SchedulerTests.cc index eb4c1678b..d72d1f36b 100644 --- a/src/programs/unit_tests/Engine/SchedulerTests.cc +++ b/src/programs/unit_tests/Engine/SchedulerTests.cc @@ -20,13 +20,12 @@ TEST_CASE("SchedulerTests") { auto q2 = new JMailbox(); auto q3 = new JMailbox(); - auto p1 = new JEventPool(jcm, 0,1,false); - auto p2 = new JEventPool(jcm, 0,1,false); + auto p1 = new JEventPool(jcm, 10, 1); auto emit_rand_ints = new RandIntArrow("emit_rand_ints", p1, q1); auto multiply_by_two = new MultByTwoArrow("multiply_by_two", q1, q2); auto subtract_one = new SubOneArrow("subtract_one", q2, q3); - auto sum_everything = new SumArrow("sum_everything", q3, p2); + auto sum_everything = new SumArrow("sum_everything", q3, p1); auto topology = std::make_shared(); @@ -105,8 +104,8 @@ TEST_CASE("SchedulerRoundRobinBehaviorTests") { auto q2 = new JMailbox(); auto q3 = new JMailbox(); - auto p1 = new JEventPool(jcm, 0,1,false); - auto p2 = new JEventPool(jcm, 0,1,false); + auto p1 = new JEventPool(jcm, 10,1); + auto p2 = new JEventPool(jcm, 10,1); auto emit_rand_ints = new RandIntArrow("emit_rand_ints", p1, q1); auto multiply_by_two = new MultByTwoArrow("multiply_by_two", q1, q2); diff --git a/src/programs/unit_tests/Topology/ArrowTests.cc b/src/programs/unit_tests/Topology/ArrowTests.cc index 1aaf84e75..b723ec23b 100644 --- a/src/programs/unit_tests/Topology/ArrowTests.cc +++ b/src/programs/unit_tests/Topology/ArrowTests.cc @@ -77,8 +77,8 @@ TEST_CASE("ArrowTests_Basic") { auto jcm = app.GetService(); JMailbox qi {2, 1, false}; - JEventPool pi {jcm, 5, 1, true}; - JEventPool pd {jcm, 5, 1, true}; + JEventPool pi {jcm, 5, 1}; + JEventPool pd {jcm, 5, 1}; JMailbox qd {2, 1, false}; TestJunctionArrow a {&qi, &pi, &pd, &qd}; diff --git a/src/programs/unit_tests/Topology/JPoolTests.cc b/src/programs/unit_tests/Topology/JPoolTests.cc index 389d2bc37..d55396570 100644 --- a/src/programs/unit_tests/Topology/JPoolTests.cc +++ b/src/programs/unit_tests/Topology/JPoolTests.cc @@ -13,7 +13,7 @@ TEST_CASE("JPoolTests_SingleLocationLimitEvents") { app.Initialize(); auto jcm = app.GetService(); - JEventPool pool(jcm, 3, 1, true); + JEventPool pool(jcm, 3, 1); auto* e = pool.get(0); REQUIRE(e != nullptr); @@ -38,49 +38,6 @@ TEST_CASE("JPoolTests_SingleLocationLimitEvents") { REQUIRE((*h)->GetEventNumber() == 5); } -TEST_CASE("JPoolTests_SingleLocationUnlimitedEvents") { - JApplication app; - app.Initialize(); - auto jcm = app.GetService(); - - JEventPool pool(jcm, 3, 1, false); - - auto* e = pool.get(0); - REQUIRE(e != nullptr); - REQUIRE((*e)->GetEventNumber() == 0); - - auto* f = pool.get(0); - std::weak_ptr f_weak = *f; - REQUIRE(f != nullptr); - REQUIRE((*f)->GetEventNumber() == 0); - - auto* g = pool.get(0); - std::weak_ptr g_weak = *g; - REQUIRE(g != nullptr); - REQUIRE((*g)->GetEventNumber() == 0); - - auto* h = pool.get(0); - // Unlike the others, h is allocated on the heap - (*h)->SetEventNumber(9); - std::weak_ptr h_weak = *h; - REQUIRE(h != nullptr); - - (*f)->SetEventNumber(5); - pool.put(f, true, 0); - // f goes back into the pool, so dtor does not get called - REQUIRE(f_weak.lock() != nullptr); - - pool.put(h, true, 0); - // h's dtor DOES get called - REQUIRE(h_weak.lock() == nullptr); - - // When we retrieve another event, it comes from the pool - // So we get what used to be f - auto* i = pool.get(0); - REQUIRE(i != nullptr); - REQUIRE((*i)->GetEventNumber() == 5); -} - } // namespace jana diff --git a/src/programs/unit_tests/Topology/TopologyTests.cc b/src/programs/unit_tests/Topology/TopologyTests.cc index fac0d83dd..f2849538b 100644 --- a/src/programs/unit_tests/Topology/TopologyTests.cc +++ b/src/programs/unit_tests/Topology/TopologyTests.cc @@ -14,16 +14,13 @@ JArrowMetrics::Status step(JArrow* arrow) { - - JArrowMetrics metrics; - arrow->execute(metrics, 0); - auto status = metrics.get_last_status(); - return status; + JArrowMetrics metrics; + arrow->execute(metrics, 0); + auto status = metrics.get_last_status(); + return status; } - - TEST_CASE("JTopology: Basic functionality") { JApplication app; app.Initialize(); @@ -33,13 +30,12 @@ TEST_CASE("JTopology: Basic functionality") { auto q2 = new JMailbox(); auto q3 = new JMailbox(); - auto p1 = new JEventPool(jcm, 0,1,false); - auto p2 = new JEventPool(jcm, 0,1,false); + auto p1 = new JEventPool(jcm, 20, 1); auto emit_rand_ints = new RandIntArrow("emit_rand_ints", p1, q1); auto multiply_by_two = new MultByTwoArrow("multiply_by_two", q1, q2); auto subtract_one = new SubOneArrow("subtract_one", q2, q3); - auto sum_everything = new SumArrow("sum_everything", q3, p2); + auto sum_everything = new SumArrow("sum_everything", q3, p1); auto topology = std::make_shared(); From 201dbed65187e49f989943ce1c1a5bef0ac5c518 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Thu, 24 Oct 2024 12:41:18 -0400 Subject: [PATCH 02/22] Clean up JEventPool Resolves confusion between max_inflight_events and event_pool_capacity when the number of locations != 1 --- src/libraries/JANA/Topology/JEventPool.h | 77 ++++++++++++------------ 1 file changed, 37 insertions(+), 40 deletions(-) diff --git a/src/libraries/JANA/Topology/JEventPool.h b/src/libraries/JANA/Topology/JEventPool.h index 7bf8fe774..6b1166f8c 100644 --- a/src/libraries/JANA/Topology/JEventPool.h +++ b/src/libraries/JANA/Topology/JEventPool.h @@ -17,12 +17,12 @@ class JEventPool { struct alignas(JANA2_CACHE_LINE_BYTES) LocalPool { std::mutex mutex; std::vector*> available_items; - std::vector> items; }; - std::unique_ptr m_pools; + std::vector> m_pools; + std::vector> m_owned_events; - size_t m_pool_size; + size_t m_max_inflight_events; size_t m_location_count; std::shared_ptr m_component_manager; @@ -31,49 +31,54 @@ class JEventPool { public: inline JEventPool(std::shared_ptr component_manager, - size_t pool_size, + size_t max_inflight_events, size_t location_count, JEventLevel level = JEventLevel::PhysicsEvent) - : m_pool_size(pool_size) + : m_max_inflight_events(max_inflight_events) , m_location_count(location_count) , m_component_manager(component_manager) , m_level(level) { assert(m_location_count >= 1); - m_pools = std::unique_ptr(new LocalPool[m_location_count]()); - - for (size_t j=0; j(); + pool->available_items.reserve(max_inflight_events); + m_pools.push_back(std::move(pool)); + } - m_pools[j].items = std::vector>(m_pool_size); // Default-construct everything in place + // Create JEvents and distribute them among the pools + m_owned_events.reserve(max_inflight_events); + for (size_t evt_idx=0; evt_idx()); + auto evt = &m_owned_events.back(); + m_component_manager->configure_event(**evt); + (*evt)->SetLevel(m_level); + m_pools[evt_idx % m_location_count]->available_items.push_back(evt); + // This only works if vector _never_ resizes } } - void configure_item(std::shared_ptr* item) { - (*item) = std::make_shared(); - m_component_manager->configure_event(**item); - item->get()->SetLevel(m_level); // This needs to happen _after_ configure_event - } - void finalize() { - for (size_t pool_idx = 0; pool_idx < m_location_count; ++pool_idx) { - for (auto& event : m_pools[pool_idx].items) { - event->Finish(); - } + for (auto& evt : m_owned_events) { + evt->Finish(); } } + std::shared_ptr* get(size_t location=0) { - assert(m_pools != nullptr); // If you hit this, you forgot to call init(). - LocalPool& pool = m_pools[location % m_location_count]; + // Note: For now this doesn't steal from another pool. In principle this means that + // all of the JEvents could end up in a single location's pool and there would be no way for + // the JEventSource in other locations to obtain fresh events. I don't think this is a problem + // in practice because the arrows always push and pull to pool's location 0, but we should + // revisit this when the time is right. + + LocalPool& pool = *(m_pools[location % m_location_count]); std::lock_guard lock(pool.mutex); if (pool.available_items.empty()) { @@ -89,33 +94,25 @@ class JEventPool { void put(std::shared_ptr* item, bool clear_event, size_t location) { - assert(m_pools != nullptr); // If you hit this, you forgot to call init(). - if (clear_event) { // Do any necessary teardown within the item itself (*item)->Clear(); } - // Consider each location starting with current one - for (size_t l = location; l lock(pool.mutex); - pool.available_items.push_back(item); - return; - } + if (pool.available_items.size() > m_max_inflight_events) { + throw JException("Event returned to already-full pool"); } - throw JException("Event returned to already-full pool"); + + std::lock_guard lock(pool.mutex); + pool.available_items.push_back(item); } size_t pop(std::shared_ptr** dest, size_t min_count, size_t max_count, size_t location) { - assert(m_pools != nullptr); // If you hit this, you forgot to call init(). - - LocalPool& pool = m_pools[location % m_location_count]; + LocalPool& pool = *(m_pools[location % m_location_count]); std::lock_guard lock(pool.mutex); size_t available_count = pool.available_items.size(); From 582d03e9d730fb9a878ec14fa2bce3c6b6be8ab8 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Thu, 24 Oct 2024 13:11:36 -0400 Subject: [PATCH 03/22] Arrows, queues, and pools pass around JEvent* --- .../SubeventExample/SubeventExample.cc | 4 +- src/libraries/JANA/JEvent.cc | 14 ++-- src/libraries/JANA/JEvent.h | 6 +- src/libraries/JANA/JEventProcessor.h | 25 ++++---- src/libraries/JANA/Topology/JArrow.h | 21 +++--- src/libraries/JANA/Topology/JEventMapArrow.cc | 18 +++--- src/libraries/JANA/Topology/JEventMapArrow.h | 2 +- src/libraries/JANA/Topology/JEventPool.h | 27 ++++---- .../JANA/Topology/JEventSourceArrow.cc | 16 ++--- .../JANA/Topology/JEventSourceArrow.h | 9 ++- src/libraries/JANA/Topology/JEventTapArrow.cc | 8 +-- src/libraries/JANA/Topology/JEventTapArrow.h | 2 +- src/libraries/JANA/Topology/JFoldArrow.h | 12 ++-- src/libraries/JANA/Topology/JPipelineArrow.h | 10 ++- src/libraries/JANA/Topology/JSubeventArrow.h | 32 +++++----- src/libraries/JANA/Topology/JUnfoldArrow.h | 26 ++++---- .../unit_tests/Components/UnfoldTests.cc | 64 +++++++++---------- .../unit_tests/Engine/ArrowActivationTests.cc | 6 +- .../unit_tests/Engine/SchedulerTests.cc | 12 ++-- .../unit_tests/Topology/ArrowTests.cc | 27 ++++---- .../unit_tests/Topology/JPoolTests.cc | 10 +-- .../unit_tests/Topology/SubeventTests.cc | 4 +- .../Topology/TestTopologyComponents.h | 28 ++++---- .../unit_tests/Topology/TopologyTests.cc | 6 +- 24 files changed, 191 insertions(+), 198 deletions(-) diff --git a/src/examples/SubeventExample/SubeventExample.cc b/src/examples/SubeventExample/SubeventExample.cc index cef877c02..8faf44c7c 100644 --- a/src/examples/SubeventExample/SubeventExample.cc +++ b/src/examples/SubeventExample/SubeventExample.cc @@ -98,8 +98,8 @@ int main() { auto topology = app.GetService(); topology->set_configure_fn([&](JTopologyBuilder& builder) { - JMailbox*> events_in; - JMailbox*> events_out; + JMailbox events_in; + JMailbox events_out; JMailbox> subevents_in; JMailbox> subevents_out; diff --git a/src/libraries/JANA/JEvent.cc b/src/libraries/JANA/JEvent.cc index cba870184..acfcb23c3 100644 --- a/src/libraries/JANA/JEvent.cc +++ b/src/libraries/JANA/JEvent.cc @@ -62,23 +62,23 @@ bool JEvent::HasParent(JEventLevel level) const { const JEvent& JEvent::GetParent(JEventLevel level) const { for (const auto& pair : mParents) { - if (pair.first == level) return *(*(pair.second)); + if (pair.first == level) return *pair.second; } throw JException("Unable to find parent at level %s", toString(level).c_str()); } -void JEvent::SetParent(std::shared_ptr* parent) { - JEventLevel level = parent->get()->GetLevel(); +void JEvent::SetParent(JEvent* parent) { + JEventLevel level = parent->GetLevel(); for (const auto& pair : mParents) { if (pair.first == level) throw JException("Event already has a parent at level %s", - toString(parent->get()->GetLevel()).c_str()); + toString(parent->GetLevel()).c_str()); } mParents.push_back({level, parent}); - parent->get()->mReferenceCount.fetch_add(1); + parent->mReferenceCount.fetch_add(1); } -std::shared_ptr* JEvent::ReleaseParent(JEventLevel level) { +JEvent* JEvent::ReleaseParent(JEventLevel level) { if (mParents.size() == 0) { throw JException("ReleaseParent failed: child has no parents!"); } @@ -88,7 +88,7 @@ std::shared_ptr* JEvent::ReleaseParent(JEventLevel level) { toString(level).c_str(), toString(pair.first).c_str()); } mParents.pop_back(); - auto remaining_refs = pair.second->get()->mReferenceCount.fetch_sub(1); + auto remaining_refs = pair.second->mReferenceCount.fetch_sub(1); if (remaining_refs < 1) { // Remember, this was fetched _before_ the last subtraction throw JException("Parent refcount has gone negative!"); } diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index 1e40ec132..4e02af10f 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -49,7 +49,7 @@ class JEvent : public std::enable_shared_from_this { bool mIsBarrierEvent = false; // Hierarchical event memory management - std::vector*>> mParents; + std::vector> mParents; std::atomic_int mReferenceCount {1}; int64_t mEventIndex = -1; @@ -89,8 +89,8 @@ class JEvent : public std::enable_shared_from_this { bool HasParent(JEventLevel level) const; const JEvent& GetParent(JEventLevel level) const; - void SetParent(std::shared_ptr* parent); - std::shared_ptr* ReleaseParent(JEventLevel level); + void SetParent(JEvent* parent); + JEvent* ReleaseParent(JEventLevel level); void Release(); // Lifecycle diff --git a/src/libraries/JANA/JEventProcessor.h b/src/libraries/JANA/JEventProcessor.h index f4ba1051e..33775a142 100644 --- a/src/libraries/JANA/JEventProcessor.h +++ b/src/libraries/JANA/JEventProcessor.h @@ -45,24 +45,24 @@ class JEventProcessor : public jana::components::JComponent, } - virtual void DoMap(const std::shared_ptr& e) { + virtual void DoMap(const JEvent& event) { if (m_callback_style == CallbackStyle::LegacyMode) { throw JException("Called DoMap() on a legacy-mode JEventProcessor"); } for (auto* input : m_inputs) { - input->PrefetchCollection(*e); + input->PrefetchCollection(event); } if (m_callback_style == CallbackStyle::ExpertMode) { - ProcessParallel(*e); + ProcessParallel(event); } else { - ProcessParallel(e->GetRunNumber(), e->GetEventNumber(), e->GetEventIndex()); + ProcessParallel(event.GetRunNumber(), event.GetEventNumber(), event.GetEventIndex()); } } - virtual void DoTap(const std::shared_ptr& e) { + virtual void DoTap(const JEvent& event) { if (m_callback_style == CallbackStyle::LegacyMode) { throw JException("Called DoReduce() on a legacy-mode JEventProcessor"); @@ -82,26 +82,23 @@ class JEventProcessor : public jana::components::JComponent, // This collection should have already been computed during DoMap() // We do this before ChangeRun() just in case we will need to pull data out of // a begin-of-run event. - input->GetCollection(*e); + input->GetCollection(event); } - auto run_number = e->GetRunNumber(); + auto run_number = event.GetRunNumber(); if (m_last_run_number != run_number) { - if (m_last_run_number != -1) { - CallWithJExceptionWrapper("JEventProcessor::EndRun", [&](){ EndRun(); }); - } for (auto* resource : m_resources) { - resource->ChangeRun(e->GetRunNumber(), m_app); + resource->ChangeRun(event.GetRunNumber(), m_app); } m_last_run_number = run_number; - CallWithJExceptionWrapper("JEventProcessor::BeginRun", [&](){ BeginRun(e); }); + CallWithJExceptionWrapper("JEventProcessor::ChangeRun", [&](){ ChangeRun(event); }); } if (m_callback_style == CallbackStyle::DeclarativeMode) { CallWithJExceptionWrapper("JEventProcessor::Process", [&](){ - Process(e->GetRunNumber(), e->GetEventNumber(), e->GetEventIndex()); + Process(event.GetRunNumber(), event.GetEventNumber(), event.GetEventIndex()); }); } else if (m_callback_style == CallbackStyle::ExpertMode) { - CallWithJExceptionWrapper("JEventProcessor::Process", [&](){ Process(*e); }); + CallWithJExceptionWrapper("JEventProcessor::Process", [&](){ Process(event); }); } m_event_count += 1; } diff --git a/src/libraries/JANA/Topology/JArrow.h b/src/libraries/JANA/Topology/JArrow.h index 09ced8640..d347417a3 100644 --- a/src/libraries/JANA/Topology/JArrow.h +++ b/src/libraries/JANA/Topology/JArrow.h @@ -19,9 +19,10 @@ struct Place; -using EventT = std::shared_ptr; - class JArrow { + friend class JScheduler; + friend class JTopologyBuilder; + private: std::string m_name; // Used for human understanding bool m_is_parallel; // Whether or not it is safe to parallelize @@ -29,13 +30,11 @@ class JArrow { bool m_is_sink; // Whether or not tnis arrow contributes to the final event count JArrowMetrics m_metrics; // Performance information accumulated over all workers - friend class JScheduler; std::vector m_listeners; // Downstream Arrows protected: // This is usable by subclasses. JLogger m_logger; - friend class JTopologyBuilder; std::vector m_places; // Will eventually supplant m_listeners public: @@ -82,7 +81,7 @@ class JArrow { }; struct Data { - std::array items; + std::array items; size_t item_count = 0; size_t reserve_count = 0; size_t location_id; @@ -107,7 +106,7 @@ struct Place { this->max_item_count = max_item_count; } - void set_queue(JMailbox* queue) { + void set_queue(JMailbox* queue) { assert(queue != nullptr); this->place_ref = queue; this->is_queue = true; @@ -122,7 +121,7 @@ struct Place { size_t get_pending() { assert(place_ref != nullptr); if (is_input && is_queue) { - auto queue = static_cast*>(place_ref); + auto queue = static_cast*>(place_ref); return queue->size(); } return 0; @@ -132,7 +131,7 @@ struct Place { assert(place_ref != nullptr); if (is_input) { // Actually pull the data if (is_queue) { - auto queue = static_cast*>(place_ref); + auto queue = static_cast*>(place_ref); data.item_count = queue->pop_and_reserve(data.items.data(), min_item_count, max_item_count, data.location_id); data.reserve_count = data.item_count; return (data.item_count >= min_item_count); @@ -148,7 +147,7 @@ struct Place { if (is_queue) { // Reserve a space on the output queue data.item_count = 0; - auto queue = static_cast*>(place_ref); + auto queue = static_cast*>(place_ref); data.reserve_count = queue->reserve(min_item_count, max_item_count, data.location_id); return (data.reserve_count >= min_item_count); } @@ -164,7 +163,7 @@ struct Place { void revert(Data& data) { assert(place_ref != nullptr); if (is_queue) { - auto queue = static_cast*>(place_ref); + auto queue = static_cast*>(place_ref); queue->push_and_unreserve(data.items.data(), data.item_count, data.reserve_count, data.location_id); } else { @@ -178,7 +177,7 @@ struct Place { size_t push(Data& data) { assert(place_ref != nullptr); if (is_queue) { - auto queue = static_cast*>(place_ref); + auto queue = static_cast*>(place_ref); queue->push_and_unreserve(data.items.data(), data.item_count, data.reserve_count, data.location_id); data.item_count = 0; data.reserve_count = 0; diff --git a/src/libraries/JANA/Topology/JEventMapArrow.cc b/src/libraries/JANA/Topology/JEventMapArrow.cc index dfefbef3d..421de9408 100644 --- a/src/libraries/JANA/Topology/JEventMapArrow.cc +++ b/src/libraries/JANA/Topology/JEventMapArrow.cc @@ -25,27 +25,27 @@ void JEventMapArrow::add_processor(JEventProcessor* processor) { m_procs.push_back(processor); } -void JEventMapArrow::process(std::shared_ptr* event, bool& success, JArrowMetrics::Status& status) { +void JEventMapArrow::process(JEvent* event, bool& success, JArrowMetrics::Status& status) { - LOG_DEBUG(m_logger) << "Executing arrow " << get_name() << " for event# " << (*event)->GetEventNumber() << LOG_END; + LOG_DEBUG(m_logger) << "Executing arrow " << get_name() << " for event# " << event->GetEventNumber() << LOG_END; for (JEventSource* source : m_sources) { - JCallGraphEntryMaker cg_entry(*(*event)->GetJCallGraphRecorder(), source->GetTypeName()); // times execution until this goes out of scope - source->Preprocess(**event); + JCallGraphEntryMaker cg_entry(*event->GetJCallGraphRecorder(), source->GetTypeName()); // times execution until this goes out of scope + source->Preprocess(*event); } for (JEventUnfolder* unfolder : m_unfolders) { - JCallGraphEntryMaker cg_entry(*(*event)->GetJCallGraphRecorder(), unfolder->GetTypeName()); // times execution until this goes out of scope - unfolder->Preprocess(**event); + JCallGraphEntryMaker cg_entry(*event->GetJCallGraphRecorder(), unfolder->GetTypeName()); // times execution until this goes out of scope + unfolder->Preprocess(*event); } for (JEventProcessor* processor : m_procs) { - JCallGraphEntryMaker cg_entry(*(*event)->GetJCallGraphRecorder(), processor->GetTypeName()); // times execution until this goes out of scope + JCallGraphEntryMaker cg_entry(*event->GetJCallGraphRecorder(), processor->GetTypeName()); // times execution until this goes out of scope if (processor->GetCallbackStyle() == JEventProcessor::CallbackStyle::LegacyMode) { - processor->DoLegacyProcess(*event); + processor->DoLegacyProcess(event->shared_from_this()); } else { processor->DoMap(*event); } } - LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " for event# " << (*event)->GetEventNumber() << LOG_END; + LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " for event# " << event->GetEventNumber() << LOG_END; success = true; status = JArrowMetrics::Status::KeepGoing; } diff --git a/src/libraries/JANA/Topology/JEventMapArrow.h b/src/libraries/JANA/Topology/JEventMapArrow.h index 742213854..563dfe438 100644 --- a/src/libraries/JANA/Topology/JEventMapArrow.h +++ b/src/libraries/JANA/Topology/JEventMapArrow.h @@ -26,7 +26,7 @@ class JEventMapArrow : public JPipelineArrow { void add_unfolder(JEventUnfolder* unfolder); void add_processor(JEventProcessor* proc); - void process(std::shared_ptr* event, bool& success, JArrowMetrics::Status& status); + void process(JEvent* event, bool& success, JArrowMetrics::Status& status); void initialize() final; void finalize() final; diff --git a/src/libraries/JANA/Topology/JEventPool.h b/src/libraries/JANA/Topology/JEventPool.h index 6b1166f8c..37dd4f4c2 100644 --- a/src/libraries/JANA/Topology/JEventPool.h +++ b/src/libraries/JANA/Topology/JEventPool.h @@ -16,7 +16,7 @@ class JEventPool { private: struct alignas(JANA2_CACHE_LINE_BYTES) LocalPool { std::mutex mutex; - std::vector*> available_items; + std::vector available_items; }; std::vector> m_pools; @@ -27,6 +27,7 @@ class JEventPool { std::shared_ptr m_component_manager; JEventLevel m_level; + JLogger m_logger; public: @@ -57,8 +58,7 @@ class JEventPool { auto evt = &m_owned_events.back(); m_component_manager->configure_event(**evt); (*evt)->SetLevel(m_level); - m_pools[evt_idx % m_location_count]->available_items.push_back(evt); - // This only works if vector _never_ resizes + m_pools[evt_idx % m_location_count]->available_items.push_back(evt->get()); } } @@ -70,7 +70,7 @@ class JEventPool { } - std::shared_ptr* get(size_t location=0) { + JEvent* get(size_t location=0) { // Note: For now this doesn't steal from another pool. In principle this means that // all of the JEvents could end up in a single location's pool and there would be no way for @@ -85,24 +85,29 @@ class JEventPool { return nullptr; } else { - std::shared_ptr* item = pool.available_items.back(); + JEvent* item = pool.available_items.back(); pool.available_items.pop_back(); return item; } } - void put(std::shared_ptr* item, bool clear_event, size_t location) { + void put(JEvent* item, bool clear_event, size_t location) { if (clear_event) { // Do any necessary teardown within the item itself - (*item)->Clear(); + item->Clear(); + } + auto use_count = item->shared_from_this().use_count(); + if (use_count > 2) { + // Use count should be 2 because there's the shared_ptr in `m_owned_events`, and there's the temporary shared_ptr created just now + throw JException("Attempted to return a JEvent to the pool while it is still being used! use_count=%d", use_count); } LocalPool& pool = *(m_pools[location]); if (pool.available_items.size() > m_max_inflight_events) { - throw JException("Event returned to already-full pool"); + throw JException("Attempted to return a JEvent to an already-full pool"); } std::lock_guard lock(pool.mutex); @@ -110,7 +115,7 @@ class JEventPool { } - size_t pop(std::shared_ptr** dest, size_t min_count, size_t max_count, size_t location) { + size_t pop(JEvent** dest, size_t min_count, size_t max_count, size_t location) { LocalPool& pool = *(m_pools[location % m_location_count]); std::lock_guard lock(pool.mutex); @@ -124,14 +129,14 @@ class JEventPool { // Return as many as we can. We aren't allowed to create any more size_t count = std::min(available_count, max_count); for (size_t i=0; i* t = pool.available_items.back(); + JEvent* t = pool.available_items.back(); pool.available_items.pop_back(); dest[i] = t; } return count; } - void push(std::shared_ptr** source, size_t count, bool clear_event, size_t location) { + void push(JEvent** source, size_t count, bool clear_event, size_t location) { for (size_t i=0; iDoNext(*event); + auto source_status = m_sources[m_current_source]->DoNext(event->shared_from_this()); if (source_status == JEventSource::Result::FailureFinished) { LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " with result FailureFinished"<< LOG_END; @@ -137,16 +137,16 @@ Event* JEventSourceArrow::process(Event* event, bool& success, JArrowMetrics::St arrow_status = JArrowMetrics::Status::ComeBackLater; return event; } - else if ((*event)->GetSequential()){ + else if (event->GetSequential()){ // Source succeeded, but returned a barrier event - LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " with result Success, holding back barrier event# " << (*event)->GetEventNumber() << LOG_END; + LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " with result Success, holding back barrier event# " << event->GetEventNumber() << LOG_END; m_pending_barrier_event = event; m_barrier_active = true; return nullptr; } else { // Source succeeded, did NOT return a barrier event - LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " with result Success, emitting event# " << (*event)->GetEventNumber() << LOG_END; + LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " with result Success, emitting event# " << event->GetEventNumber() << LOG_END; success = true; arrow_status = JArrowMetrics::Status::KeepGoing; return event; diff --git a/src/libraries/JANA/Topology/JEventSourceArrow.h b/src/libraries/JANA/Topology/JEventSourceArrow.h index a4e5957ce..2ee89821d 100644 --- a/src/libraries/JANA/Topology/JEventSourceArrow.h +++ b/src/libraries/JANA/Topology/JEventSourceArrow.h @@ -5,14 +5,13 @@ #pragma once #include -using Event = std::shared_ptr; class JEventSourceArrow : public JArrow { private: std::vector m_sources; size_t m_current_source = 0; bool m_barrier_active = false; - std::shared_ptr* m_pending_barrier_event = nullptr; + JEvent* m_pending_barrier_event = nullptr; Place m_input {this, true, 1, 1}; Place m_output {this, false, 1, 1}; @@ -20,13 +19,13 @@ class JEventSourceArrow : public JArrow { public: JEventSourceArrow(std::string name, std::vector sources); - void set_input(JMailbox* queue) { + void set_input(JMailbox* queue) { m_input.set_queue(queue); } void set_input(JEventPool* pool) { m_input.set_pool(pool); } - void set_output(JMailbox* queue) { + void set_output(JMailbox* queue) { m_output.set_queue(queue); } void set_output(JEventPool* pool) { @@ -36,6 +35,6 @@ class JEventSourceArrow : public JArrow { void initialize() final; void finalize() final; void execute(JArrowMetrics& result, size_t location_id) final; - Event* process(Event* event, bool& success, JArrowMetrics::Status& status); + JEvent* process(JEvent* event, bool& success, JArrowMetrics::Status& status); }; diff --git a/src/libraries/JANA/Topology/JEventTapArrow.cc b/src/libraries/JANA/Topology/JEventTapArrow.cc index 7be832c9d..e66d9b522 100644 --- a/src/libraries/JANA/Topology/JEventTapArrow.cc +++ b/src/libraries/JANA/Topology/JEventTapArrow.cc @@ -15,16 +15,16 @@ void JEventTapArrow::add_processor(JEventProcessor* proc) { m_procs.push_back(proc); } -void JEventTapArrow::process(std::shared_ptr* event, bool& success, JArrowMetrics::Status& status) { +void JEventTapArrow::process(JEvent* event, bool& success, JArrowMetrics::Status& status) { - LOG_DEBUG(m_logger) << "Executing arrow " << get_name() << " for event# " << (*event)->GetEventNumber() << LOG_END; + LOG_DEBUG(m_logger) << "Executing arrow " << get_name() << " for event# " << event->GetEventNumber() << LOG_END; for (JEventProcessor* proc : m_procs) { - JCallGraphEntryMaker cg_entry(*(*event)->GetJCallGraphRecorder(), proc->GetTypeName()); // times execution until this goes out of scope + JCallGraphEntryMaker cg_entry(*event->GetJCallGraphRecorder(), proc->GetTypeName()); // times execution until this goes out of scope if (proc->GetCallbackStyle() != JEventProcessor::CallbackStyle::LegacyMode) { proc->DoTap(*event); } } - LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " for event# " << (*event)->GetEventNumber() << LOG_END; + LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " for event# " << event->GetEventNumber() << LOG_END; success = true; status = JArrowMetrics::Status::KeepGoing; } diff --git a/src/libraries/JANA/Topology/JEventTapArrow.h b/src/libraries/JANA/Topology/JEventTapArrow.h index dc2a922c7..f2020dea6 100644 --- a/src/libraries/JANA/Topology/JEventTapArrow.h +++ b/src/libraries/JANA/Topology/JEventTapArrow.h @@ -18,7 +18,7 @@ class JEventTapArrow : public JPipelineArrow { JEventTapArrow(std::string name); void add_processor(JEventProcessor* proc); - void process(std::shared_ptr* event, bool& success, JArrowMetrics::Status& status); + void process(JEvent* event, bool& success, JArrowMetrics::Status& status); void initialize() final; void finalize() final; }; diff --git a/src/libraries/JANA/Topology/JFoldArrow.h b/src/libraries/JANA/Topology/JFoldArrow.h index e662aa444..47f17a95f 100644 --- a/src/libraries/JANA/Topology/JFoldArrow.h +++ b/src/libraries/JANA/Topology/JFoldArrow.h @@ -8,8 +8,6 @@ class JFoldArrow : public JArrow { private: - using EventT = std::shared_ptr; - // TODO: Support user-provided folders // JEventFolder* m_folder = nullptr; @@ -36,12 +34,12 @@ class JFoldArrow : public JArrow { { } - void attach_child_in(JMailbox* child_in) { + void attach_child_in(JMailbox* child_in) { m_child_in.place_ref = child_in; m_child_in.is_queue = true; } - void attach_child_out(JMailbox* child_out) { + void attach_child_out(JMailbox* child_out) { m_child_out.place_ref = child_out; m_child_out.is_queue = true; } @@ -57,7 +55,7 @@ class JFoldArrow : public JArrow { } - void attach_parent_out(JMailbox* parent_out) { + void attach_parent_out(JMailbox* parent_out) { m_parent_out.place_ref = parent_out; m_parent_out.is_queue = true; } @@ -125,12 +123,12 @@ class JFoldArrow : public JArrow { auto child = child_in_data.items[0]; child_in_data.items[0] = nullptr; child_in_data.item_count = 0; - if (child->get()->GetLevel() != m_child_level) { + if (child->GetLevel() != m_child_level) { throw JException("JFoldArrow received a child with the wrong event level"); } // TODO: Call folders here - auto* parent = child->get()->ReleaseParent(m_parent_level); + auto* parent = child->ReleaseParent(m_parent_level); // Put child on the output queue child_out_data.items[0] = child; diff --git a/src/libraries/JANA/Topology/JPipelineArrow.h b/src/libraries/JANA/Topology/JPipelineArrow.h index f48ea14f1..225f4e995 100644 --- a/src/libraries/JANA/Topology/JPipelineArrow.h +++ b/src/libraries/JANA/Topology/JPipelineArrow.h @@ -8,7 +8,6 @@ #include #include -using MessageT = std::shared_ptr; template class JPipelineArrow : public JArrow { @@ -24,13 +23,13 @@ class JPipelineArrow : public JArrow { : JArrow(std::move(name), is_parallel, is_source, is_sink) { } - void set_input(JMailbox* queue) { + void set_input(JMailbox* queue) { m_input.set_queue(queue); } void set_input(JEventPool* pool) { m_input.set_pool(pool); } - void set_output(JMailbox* queue) { + void set_output(JMailbox* queue) { m_output.set_queue(queue); } void set_output(JEventPool* pool) { @@ -48,8 +47,7 @@ class JPipelineArrow : public JArrow { if (!success) { m_input.revert(in_data); m_output.revert(out_data); - // TODO: Test that revert works properly - + auto end_total_time = std::chrono::steady_clock::now(); result.update(JArrowMetrics::Status::ComeBackLater, 0, 1, std::chrono::milliseconds(0), end_total_time - start_total_time); return; @@ -58,7 +56,7 @@ class JPipelineArrow : public JArrow { bool process_succeeded = true; JArrowMetrics::Status process_status = JArrowMetrics::Status::KeepGoing; assert(in_data.item_count == 1); - MessageT* event = in_data.items[0]; + JEvent* event = in_data.items[0]; auto start_processing_time = std::chrono::steady_clock::now(); static_cast(this)->process(event, process_succeeded, process_status); diff --git a/src/libraries/JANA/Topology/JSubeventArrow.h b/src/libraries/JANA/Topology/JSubeventArrow.h index 8aeab67db..8380089f1 100644 --- a/src/libraries/JANA/Topology/JSubeventArrow.h +++ b/src/libraries/JANA/Topology/JSubeventArrow.h @@ -36,12 +36,12 @@ struct JSubeventProcessor { template struct SubeventWrapper { - std::shared_ptr* parent; + JEvent* parent; SubeventT* data; size_t id; size_t total; - SubeventWrapper(std::shared_ptr* parent, SubeventT* data, size_t id, size_t total) + SubeventWrapper(JEvent* parent, SubeventT* data, size_t id, size_t total) : parent(std::move(parent)) , data(data) , id(id) @@ -69,12 +69,12 @@ class JSubeventArrow : public JArrow { template class JSplitArrow : public JArrow { JSubeventProcessor* m_processor; - JMailbox*>* m_inbox; + JMailbox* m_inbox; JMailbox>* m_outbox; public: JSplitArrow(std::string name, JSubeventProcessor* processor, - JMailbox*>* inbox, + JMailbox* inbox, JMailbox>* outbox) : JArrow(name, true, false, false), m_processor(processor), m_inbox(inbox), m_outbox(outbox) { } @@ -87,13 +87,13 @@ template class JMergeArrow : public JArrow { JSubeventProcessor* m_processor; JMailbox>* m_inbox; - JMailbox*>* m_outbox; - std::map*, size_t> m_in_progress; + JMailbox* m_outbox; + std::map m_in_progress; public: JMergeArrow(std::string name, JSubeventProcessor* processor, JMailbox>* inbox, - JMailbox*>* outbox) + JMailbox* outbox) : JArrow(name, false, false, false), m_processor(processor), m_inbox(inbox), m_outbox(outbox) { } @@ -105,11 +105,11 @@ class JMergeArrow : public JArrow { template void JSplitArrow::execute(JArrowMetrics& result, size_t location_id) { - using InQueue = JMailbox*>; + using InQueue = JMailbox; using OutQueue = JMailbox>; auto start_total_time = std::chrono::steady_clock::now(); - std::shared_ptr* event = nullptr; + JEvent* event = nullptr; bool success; size_t reserved_size = m_outbox->reserve(1); size_t actual_size = reserved_size; @@ -122,7 +122,7 @@ void JSplitArrow::execute(JArrowMetrics& result, size_t locatio if (success) { // Construct prereqs - std::vector originals = (*event)->Get(m_processor->inputTag); + std::vector originals = event->Get(m_processor->inputTag); size_t i = 1; actual_size = originals.size(); @@ -195,7 +195,7 @@ void JSubeventArrow::execute(JArrowMetrics& result, size_t loca template void JMergeArrow::execute(JArrowMetrics& result, size_t location_id) { using InQueue = JMailbox>; - using OutQueue = JMailbox*>; + using OutQueue = JMailbox; auto start_total_time = std::chrono::steady_clock::now(); @@ -205,16 +205,16 @@ void JMergeArrow::execute(JArrowMetrics& result, size_t locatio auto in_status = m_inbox->pop(inputs, downstream_accepts, location_id); auto start_latency_time = std::chrono::steady_clock::now(); - std::vector*> outputs; + std::vector outputs; for (const auto& input : inputs) { - LOG_TRACE(m_logger) << "JMergeArrow: Processing input with parent=" << input.parent << ", evt=" << (*(input.parent))->GetEventNumber() << ", sub=" << input.id << " and total=" << input.total << LOG_END; + LOG_TRACE(m_logger) << "JMergeArrow: Processing input with parent=" << input.parent << ", evt=" << input.parent->GetEventNumber() << ", sub=" << input.id << " and total=" << input.total << LOG_END; // Problem: Are we sure we are updating the event in a way which is effectively thread-safe? // Should we be doing this insert, or should the caller? - (*(input.parent))->template Insert(input.data); + input.parent->template Insert(input.data); if (input.total == 1) { // Goes straight into "ready" outputs.push_back(input.parent); - LOG_TRACE(m_logger) << "JMergeArrow: Finished parent=" << input.parent << ", evt=" << (*(input.parent))->GetEventNumber() << LOG_END; + LOG_TRACE(m_logger) << "JMergeArrow: Finished parent=" << input.parent << ", evt=" << input.parent->GetEventNumber() << LOG_END; } else { auto pair = m_in_progress.find(input.parent); @@ -228,7 +228,7 @@ void JMergeArrow::execute(JArrowMetrics& result, size_t locatio else if (pair->second == 1) { pair->second -= 1; outputs.push_back(input.parent); - LOG_TRACE(m_logger) << "JMergeArrow: Finished parent=" << input.parent << ", evt=" << (*(input.parent))->GetEventNumber() << LOG_END; + LOG_TRACE(m_logger) << "JMergeArrow: Finished parent=" << input.parent << ", evt=" << input.parent->GetEventNumber() << LOG_END; } else { pair->second -= 1; diff --git a/src/libraries/JANA/Topology/JUnfoldArrow.h b/src/libraries/JANA/Topology/JUnfoldArrow.h index a5befde01..31929dc1b 100644 --- a/src/libraries/JANA/Topology/JUnfoldArrow.h +++ b/src/libraries/JANA/Topology/JUnfoldArrow.h @@ -10,7 +10,7 @@ class JUnfoldArrow : public JArrow { private: JEventUnfolder* m_unfolder = nullptr; - EventT* m_parent_event = nullptr; + JEvent* m_parent_event = nullptr; bool m_ready_to_fetch_parent = true; Place m_parent_in; @@ -32,7 +32,7 @@ class JUnfoldArrow : public JArrow { } - void attach_parent_in(JMailbox* parent_in) { + void attach_parent_in(JMailbox* parent_in) { m_parent_in.place_ref = parent_in; m_parent_in.is_queue = true; } @@ -42,12 +42,12 @@ class JUnfoldArrow : public JArrow { m_child_in.is_queue = false; } - void attach_child_in(JMailbox* child_in) { + void attach_child_in(JMailbox* child_in) { m_child_in.place_ref = child_in; m_child_in.is_queue = true; } - void attach_child_out(JMailbox* child_out) { + void attach_child_out(JMailbox* child_out) { m_child_out.place_ref = child_out; m_child_out.is_queue = true; } @@ -131,26 +131,26 @@ class JUnfoldArrow : public JArrow { if (m_parent_event == nullptr) { throw JException("Attempting to unfold without a valid parent event"); } - if (m_parent_event->get()->GetLevel() != m_unfolder->GetLevel()) { - throw JException("JUnfolder: Expected parent with level %d, got %d", m_unfolder->GetLevel(), m_parent_event->get()->GetLevel()); + if (m_parent_event->GetLevel() != m_unfolder->GetLevel()) { + throw JException("JUnfolder: Expected parent with level %d, got %d", m_unfolder->GetLevel(), m_parent_event->GetLevel()); } - if (child->get()->GetLevel() != m_unfolder->GetChildLevel()) { - throw JException("JUnfolder: Expected child with level %d, got %d", m_unfolder->GetChildLevel(), child->get()->GetLevel()); + if (child->GetLevel() != m_unfolder->GetChildLevel()) { + throw JException("JUnfolder: Expected child with level %d, got %d", m_unfolder->GetChildLevel(), child->GetLevel()); } - auto status = m_unfolder->DoUnfold(*(m_parent_event->get()), *(child->get())); + auto status = m_unfolder->DoUnfold(*m_parent_event, *child); // Join always succeeds (for now) - child->get()->SetParent(m_parent_event); + child->SetParent(m_parent_event); - LOG_DEBUG(m_logger) << "Unfold succeeded: Parent event = " << m_parent_event->get()->GetEventNumber() << ", child event = " << child->get()->GetEventNumber() << LOG_END; + LOG_DEBUG(m_logger) << "Unfold succeeded: Parent event = " << m_parent_event->GetEventNumber() << ", child event = " << child->GetEventNumber() << LOG_END; // TODO: We'll need something more complicated for the streaming join case if (status == JEventUnfolder::Result::NextChildNextParent || status == JEventUnfolder::Result::KeepChildNextParent) { - LOG_DEBUG(m_logger) << "Unfold finished with parent event = " << m_parent_event->get()->GetEventNumber() << LOG_END; + LOG_DEBUG(m_logger) << "Unfold finished with parent event = " << m_parent_event->GetEventNumber() << LOG_END; m_ready_to_fetch_parent = true; - m_parent_event->get()->Release(); + m_parent_event->Release(); m_parent_event = nullptr; m_parent_in.min_item_count = 1; m_parent_in.max_item_count = 1; diff --git a/src/programs/unit_tests/Components/UnfoldTests.cc b/src/programs/unit_tests/Components/UnfoldTests.cc index 3fa1341c9..371d095cd 100644 --- a/src/programs/unit_tests/Components/UnfoldTests.cc +++ b/src/programs/unit_tests/Components/UnfoldTests.cc @@ -6,8 +6,6 @@ namespace jana { namespace unfoldtests { -using EventT = std::shared_ptr; - struct TestUnfolder : public JEventUnfolder { mutable std::vector preprocessed_event_nrs; @@ -46,16 +44,16 @@ TEST_CASE("UnfoldTests_Basic") { app.Initialize(); auto jcm = app.GetService(); - JEventPool parent_pool {jcm, 5, 1, JEventLevel::Timeslice}; // size=5, locations=1, limit_total_events_in_flight=true + JEventPool parent_pool {jcm, 5, 1, JEventLevel::Timeslice}; JEventPool child_pool {jcm, 5, 1, JEventLevel::PhysicsEvent}; - JMailbox parent_queue {3}; // size - JMailbox child_queue {3}; + JMailbox parent_queue {3}; // size + JMailbox child_queue {3}; auto ts1 = parent_pool.get(); - (*ts1)->SetEventNumber(17); + ts1->SetEventNumber(17); auto ts2 = parent_pool.get(); - (*ts2)->SetEventNumber(28); + ts2->SetEventNumber(28); parent_queue.try_push(&ts1, 1); parent_queue.try_push(&ts2, 1); @@ -89,13 +87,13 @@ TEST_CASE("FoldArrowTests") { // We only use these to obtain preconfigured JEvents - JEventPool parent_pool {jcm, 5, 1, JEventLevel::Timeslice}; // size=5, locations=1, limit_total_events_in_flight=true + JEventPool parent_pool {jcm, 5, 1, JEventLevel::Timeslice}; JEventPool child_pool {jcm, 5, 1, JEventLevel::PhysicsEvent}; // We set up our test cases by putting events on these queues - JMailbox*> child_in; - JMailbox*> child_out; - JMailbox*> parent_out; + JMailbox child_in; + JMailbox child_out; + JMailbox parent_out; JFoldArrow arrow("sut", JEventLevel::Timeslice, JEventLevel::PhysicsEvent); arrow.attach_child_in(&child_in); @@ -107,25 +105,25 @@ TEST_CASE("FoldArrowTests") { SECTION("One-to-one relationship between timeslices and events") { auto ts1 = parent_pool.get(); - (*ts1)->SetEventNumber(17); - REQUIRE(ts1->get()->GetLevel() == JEventLevel::Timeslice); + ts1->SetEventNumber(17); + REQUIRE(ts1->GetLevel() == JEventLevel::Timeslice); auto ts2 = parent_pool.get(); - (*ts2)->SetEventNumber(28); + ts2->SetEventNumber(28); auto evt1 = child_pool.get(); - (*evt1)->SetEventNumber(111); + evt1->SetEventNumber(111); auto evt2 = child_pool.get(); - (*evt2)->SetEventNumber(112); + evt2->SetEventNumber(112); - evt1->get()->SetParent(ts1); - ts1->get()->Release(); // One-to-one + evt1->SetParent(ts1); + ts1->Release(); // One-to-one child_in.try_push(&evt1, 1, 0); - evt2->get()->SetParent(ts2); - ts2->get()->Release(); // One-to-one + evt2->SetParent(ts2); + ts2->Release(); // One-to-one child_in.try_push(&evt2, 1, 0); arrow.execute(metrics, 0); @@ -140,32 +138,32 @@ TEST_CASE("FoldArrowTests") { SECTION("One-to-two relationship between timeslices and events") { auto ts1 = parent_pool.get(); - (*ts1)->SetEventNumber(17); - REQUIRE(ts1->get()->GetLevel() == JEventLevel::Timeslice); + ts1->SetEventNumber(17); + REQUIRE(ts1->GetLevel() == JEventLevel::Timeslice); auto ts2 = parent_pool.get(); - (*ts2)->SetEventNumber(28); + ts2->SetEventNumber(28); auto evt1 = child_pool.get(); - (*evt1)->SetEventNumber(111); + evt1->SetEventNumber(111); auto evt2 = child_pool.get(); - (*evt2)->SetEventNumber(112); + evt2->SetEventNumber(112); auto evt3 = child_pool.get(); - (*evt3)->SetEventNumber(113); + evt3->SetEventNumber(113); auto evt4 = child_pool.get(); - (*evt4)->SetEventNumber(114); + evt4->SetEventNumber(114); - evt1->get()->SetParent(ts1); - evt2->get()->SetParent(ts1); - ts1->get()->Release(); // One-to-two + evt1->SetParent(ts1); + evt2->SetParent(ts1); + ts1->Release(); // One-to-two - evt3->get()->SetParent(ts2); - evt4->get()->SetParent(ts2); - ts2->get()->Release(); // One-to-two + evt3->SetParent(ts2); + evt4->SetParent(ts2); + ts2->Release(); // One-to-two child_in.try_push(&evt1, 1, 0); child_in.try_push(&evt2, 1, 0); diff --git a/src/programs/unit_tests/Engine/ArrowActivationTests.cc b/src/programs/unit_tests/Engine/ArrowActivationTests.cc index bb1d5679b..4bd491d91 100644 --- a/src/programs/unit_tests/Engine/ArrowActivationTests.cc +++ b/src/programs/unit_tests/Engine/ArrowActivationTests.cc @@ -21,9 +21,9 @@ TEST_CASE("ArrowActivationTests") { app.Initialize(); auto jcm = app.GetService(); - auto q1 = new JMailbox(); - auto q2 = new JMailbox(); - auto q3 = new JMailbox(); + auto q1 = new JMailbox(); + auto q2 = new JMailbox(); + auto q3 = new JMailbox(); auto p1 = new JEventPool(jcm, 10,1); auto p2 = new JEventPool(jcm, 10,1); diff --git a/src/programs/unit_tests/Engine/SchedulerTests.cc b/src/programs/unit_tests/Engine/SchedulerTests.cc index d72d1f36b..fdcde12f5 100644 --- a/src/programs/unit_tests/Engine/SchedulerTests.cc +++ b/src/programs/unit_tests/Engine/SchedulerTests.cc @@ -16,9 +16,9 @@ TEST_CASE("SchedulerTests") { app.Initialize(); auto jcm = app.GetService(); - auto q1 = new JMailbox(); - auto q2 = new JMailbox(); - auto q3 = new JMailbox(); + auto q1 = new JMailbox(); + auto q2 = new JMailbox(); + auto q3 = new JMailbox(); auto p1 = new JEventPool(jcm, 10, 1); @@ -100,9 +100,9 @@ TEST_CASE("SchedulerRoundRobinBehaviorTests") { app.Initialize(); auto jcm = app.GetService(); - auto q1 = new JMailbox(); - auto q2 = new JMailbox(); - auto q3 = new JMailbox(); + auto q1 = new JMailbox(); + auto q2 = new JMailbox(); + auto q3 = new JMailbox(); auto p1 = new JEventPool(jcm, 10,1); auto p2 = new JEventPool(jcm, 10,1); diff --git a/src/programs/unit_tests/Topology/ArrowTests.cc b/src/programs/unit_tests/Topology/ArrowTests.cc index b723ec23b..555c14d97 100644 --- a/src/programs/unit_tests/Topology/ArrowTests.cc +++ b/src/programs/unit_tests/Topology/ArrowTests.cc @@ -12,13 +12,12 @@ struct ArrowTestData { double y; }; -using EventT = std::shared_ptr; struct TestJunctionArrow : public JJunctionArrow { - TestJunctionArrow(JMailbox* qi, + TestJunctionArrow(JMailbox* qi, JEventPool* pi, JEventPool* pd, - JMailbox* qd) + JMailbox* qd) : JJunctionArrow("testjunctionarrow", false, false, true) { first_input.set_queue(qi); @@ -42,22 +41,22 @@ struct TestJunctionArrow : public JJunctionArrow { REQUIRE(output_double.item_count == 0); REQUIRE(output_double.reserve_count == 1); - EventT* x_event = input_int.items[0]; + JEvent* x_event = input_int.items[0]; input_int.items[0] = nullptr; input_int.item_count = 0; // TODO: Maybe user shouldn't be allowed to modify reserve_count at all // TODO Maybe user should only be allowed to push and pull from ... range...? - EventT* y_event = input_double.items[0]; + JEvent* y_event = input_double.items[0]; input_double.items[0] = nullptr; input_double.item_count = 0; - auto data = (*x_event)->Get(); + auto data = x_event->Get(); int x = data.at(0)->x; // Do something useful here double y = x + 22.2; - (*y_event)->Insert(new ArrowTestData{.x = x, .y = y}); + y_event->Insert(new ArrowTestData{.x = x, .y = y}); output_int.items[0] = x_event; output_int.item_count = 1; @@ -76,26 +75,26 @@ TEST_CASE("ArrowTests_Basic") { app.Initialize(); auto jcm = app.GetService(); - JMailbox qi {2, 1, false}; + JMailbox qi {2, 1, false}; JEventPool pi {jcm, 5, 1}; JEventPool pd {jcm, 5, 1}; - JMailbox qd {2, 1, false}; + JMailbox qd {2, 1, false}; TestJunctionArrow a {&qi, &pi, &pd, &qd}; - EventT* x = nullptr; + JEvent* x = nullptr; pi.pop(&x, 1, 1, 0); REQUIRE(x != nullptr); - REQUIRE((*x)->GetEventNumber() == 0); - (*x)->Insert(new ArrowTestData {.x = 100, .y=0}); + REQUIRE(x->GetEventNumber() == 0); + x->Insert(new ArrowTestData {.x = 100, .y=0}); qi.push_and_unreserve(&x, 1, 0, 0); JArrowMetrics m; a.execute(m, 0); - EventT* y; + JEvent* y; qd.pop_and_reserve(&y, 1, 1, 0); - auto data = (*y)->Get(); + auto data = y->Get(); REQUIRE(data.at(0)->y == 122.2); } diff --git a/src/programs/unit_tests/Topology/JPoolTests.cc b/src/programs/unit_tests/Topology/JPoolTests.cc index d55396570..ed4fc7776 100644 --- a/src/programs/unit_tests/Topology/JPoolTests.cc +++ b/src/programs/unit_tests/Topology/JPoolTests.cc @@ -17,25 +17,25 @@ TEST_CASE("JPoolTests_SingleLocationLimitEvents") { auto* e = pool.get(0); REQUIRE(e != nullptr); - REQUIRE((*e)->GetEventNumber() == 0); // Will segfault if not initialized + REQUIRE(e->GetEventNumber() == 0); // Will segfault if not initialized auto* f = pool.get(0); REQUIRE(f != nullptr); - REQUIRE((*f)->GetEventNumber() == 0); + REQUIRE(f->GetEventNumber() == 0); auto* g = pool.get(0); REQUIRE(g != nullptr); - REQUIRE((*g)->GetEventNumber() == 0); + REQUIRE(g->GetEventNumber() == 0); auto* h = pool.get(0); REQUIRE(h == nullptr); - (*f)->SetEventNumber(5); + f->SetEventNumber(5); pool.put(f, true, 0); h = pool.get(0); REQUIRE(h != nullptr); - REQUIRE((*h)->GetEventNumber() == 5); + REQUIRE(h->GetEventNumber() == 5); } diff --git a/src/programs/unit_tests/Topology/SubeventTests.cc b/src/programs/unit_tests/Topology/SubeventTests.cc index f0881fbab..c35159b20 100644 --- a/src/programs/unit_tests/Topology/SubeventTests.cc +++ b/src/programs/unit_tests/Topology/SubeventTests.cc @@ -47,8 +47,8 @@ TEST_CASE("Create subevent processor") { TEST_CASE("Basic subevent arrow functionality") { MyProcessor processor; - JMailbox*> events_in; - JMailbox*> events_out; + JMailbox events_in; + JMailbox events_out; JMailbox> subevents_in; JMailbox> subevents_out; diff --git a/src/programs/unit_tests/Topology/TestTopologyComponents.h b/src/programs/unit_tests/Topology/TestTopologyComponents.h index c4df0e46f..b9a6e6bcb 100644 --- a/src/programs/unit_tests/Topology/TestTopologyComponents.h +++ b/src/programs/unit_tests/Topology/TestTopologyComponents.h @@ -20,13 +20,13 @@ struct RandIntArrow : public JPipelineArrow { size_t emit_count = 0; // How many emitted so far int emit_sum = 0; // Sum of all ints emitted so far - RandIntArrow(std::string name, JEventPool* pool, JMailbox* output_queue) + RandIntArrow(std::string name, JEventPool* pool, JMailbox* output_queue) : JPipelineArrow(name, false, true, false) { this->set_input(pool); this->set_output(output_queue); } - void process(EventT* event, bool& success, JArrowMetrics::Status& status) { + void process(JEvent* event, bool& success, JArrowMetrics::Status& status) { if (emit_count >= emit_limit) { success = false; @@ -36,7 +36,7 @@ struct RandIntArrow : public JPipelineArrow { auto data = new EventData {7}; - (*event)->Insert(data, "first"); + event->Insert(data, "first"); emit_sum += data->x; emit_count += 1; @@ -50,17 +50,17 @@ struct RandIntArrow : public JPipelineArrow { struct MultByTwoArrow : public JPipelineArrow { - MultByTwoArrow(std::string name, JMailbox* input_queue, JMailbox* output_queue) + MultByTwoArrow(std::string name, JMailbox* input_queue, JMailbox* output_queue) : JPipelineArrow(name, true, false, false) { this->set_input(input_queue); this->set_output(output_queue); } - void process(EventT* event, bool& success, JArrowMetrics::Status& status) { - auto prev = (*event)->Get("first"); + void process(JEvent* event, bool& success, JArrowMetrics::Status& status) { + auto prev = event->Get("first"); auto x = prev.at(0)->x; auto next = new EventData { .x=x, .y=x*2.0 }; - (*event)->Insert(next, "second"); + event->Insert(next, "second"); success = true; status = JArrowMetrics::Status::KeepGoing; } @@ -68,19 +68,19 @@ struct MultByTwoArrow : public JPipelineArrow { struct SubOneArrow : public JPipelineArrow { - SubOneArrow(std::string name, JMailbox* input_queue, JMailbox* output_queue) + SubOneArrow(std::string name, JMailbox* input_queue, JMailbox* output_queue) : JPipelineArrow(name, true, false, false) { this->set_input(input_queue); this->set_output(output_queue); } - void process(EventT* event, bool& success, JArrowMetrics::Status& status) { - auto prev = (*event)->Get("second"); + void process(JEvent* event, bool& success, JArrowMetrics::Status& status) { + auto prev = event->Get("second"); auto x = prev.at(0)->x; auto y = prev.at(0)->y; auto z = y - 1; auto next = new EventData { .x=x, .y=y, .z=z }; - (*event)->Insert(next, "third"); + event->Insert(next, "third"); success = true; status = JArrowMetrics::Status::KeepGoing; } @@ -90,14 +90,14 @@ struct SumArrow : public JPipelineArrow { double sum = 0; - SumArrow(std::string name, JMailbox* input_queue, JEventPool* pool) + SumArrow(std::string name, JMailbox* input_queue, JEventPool* pool) : JPipelineArrow(name, false, false, true) { this->set_input(input_queue); this->set_output(pool); } - void process(EventT* event, bool& success, JArrowMetrics::Status& status) { - auto prev = (*event)->Get("third"); + void process(JEvent* event, bool& success, JArrowMetrics::Status& status) { + auto prev = event->Get("third"); auto z = prev.at(0)->z; sum += z; success = true; diff --git a/src/programs/unit_tests/Topology/TopologyTests.cc b/src/programs/unit_tests/Topology/TopologyTests.cc index f2849538b..8e0b2af91 100644 --- a/src/programs/unit_tests/Topology/TopologyTests.cc +++ b/src/programs/unit_tests/Topology/TopologyTests.cc @@ -26,9 +26,9 @@ TEST_CASE("JTopology: Basic functionality") { app.Initialize(); auto jcm = app.GetService(); - auto q1 = new JMailbox(); - auto q2 = new JMailbox(); - auto q3 = new JMailbox(); + auto q1 = new JMailbox(); + auto q2 = new JMailbox(); + auto q3 = new JMailbox(); auto p1 = new JEventPool(jcm, 20, 1); From 977da32dc81b7b1abec6321e7542d09802dfec4d Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 25 Oct 2024 11:47:26 -0400 Subject: [PATCH 04/22] Disable queue reservations This is two fewer locks on every attempt to execute an arrow --- src/libraries/JANA/Topology/JArrow.h | 12 +++++++----- src/programs/unit_tests/Topology/ArrowTests.cc | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libraries/JANA/Topology/JArrow.h b/src/libraries/JANA/Topology/JArrow.h index d347417a3..578496bf9 100644 --- a/src/libraries/JANA/Topology/JArrow.h +++ b/src/libraries/JANA/Topology/JArrow.h @@ -132,8 +132,8 @@ struct Place { if (is_input) { // Actually pull the data if (is_queue) { auto queue = static_cast*>(place_ref); - data.item_count = queue->pop_and_reserve(data.items.data(), min_item_count, max_item_count, data.location_id); - data.reserve_count = data.item_count; + data.item_count = queue->pop(data.items.data(), min_item_count, max_item_count, data.location_id); + //data.item_count = queue->pop_and_reserve(data.items.data(), min_item_count, max_item_count, data.location_id); return (data.item_count >= min_item_count); } else { @@ -147,9 +147,9 @@ struct Place { if (is_queue) { // Reserve a space on the output queue data.item_count = 0; - auto queue = static_cast*>(place_ref); - data.reserve_count = queue->reserve(min_item_count, max_item_count, data.location_id); - return (data.reserve_count >= min_item_count); + //auto queue = static_cast*>(place_ref); + data.reserve_count = 0; //queue->reserve(min_item_count, max_item_count, data.location_id); + return true; //(data.reserve_count >= min_item_count); } else { // No need to reserve on pool -- either there is space or limit_events_in_flight=false @@ -164,6 +164,7 @@ struct Place { assert(place_ref != nullptr); if (is_queue) { auto queue = static_cast*>(place_ref); + assert(data.reserve_count == 0); queue->push_and_unreserve(data.items.data(), data.item_count, data.reserve_count, data.location_id); } else { @@ -178,6 +179,7 @@ struct Place { assert(place_ref != nullptr); if (is_queue) { auto queue = static_cast*>(place_ref); + assert(data.reserve_count == 0); queue->push_and_unreserve(data.items.data(), data.item_count, data.reserve_count, data.location_id); data.item_count = 0; data.reserve_count = 0; diff --git a/src/programs/unit_tests/Topology/ArrowTests.cc b/src/programs/unit_tests/Topology/ArrowTests.cc index 555c14d97..2ec71ac7d 100644 --- a/src/programs/unit_tests/Topology/ArrowTests.cc +++ b/src/programs/unit_tests/Topology/ArrowTests.cc @@ -33,13 +33,13 @@ struct TestJunctionArrow : public JJunctionArrow { std::cout << "Hello from process" << std::endl; REQUIRE(input_int.item_count == 1); - REQUIRE(input_int.reserve_count == 1); + REQUIRE(input_int.reserve_count == 0); // Would be 1 REQUIRE(output_int.item_count == 0); REQUIRE(output_int.reserve_count == 0); REQUIRE(input_double.item_count == 1); REQUIRE(input_double.reserve_count == 0); REQUIRE(output_double.item_count == 0); - REQUIRE(output_double.reserve_count == 1); + REQUIRE(output_double.reserve_count == 0); // Would be 1 JEvent* x_event = input_int.items[0]; input_int.items[0] = nullptr; From a30449f403583ee905211557b01de91e9bfb7747 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 25 Oct 2024 11:58:51 -0400 Subject: [PATCH 05/22] WIP: Refactor JArrow::execute --- src/libraries/JANA/Topology/JArrow.h | 6 ++++++ src/libraries/JANA/Topology/JEventMapArrow.cc | 11 ++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/libraries/JANA/Topology/JArrow.h b/src/libraries/JANA/Topology/JArrow.h index 578496bf9..461fbe2db 100644 --- a/src/libraries/JANA/Topology/JArrow.h +++ b/src/libraries/JANA/Topology/JArrow.h @@ -18,6 +18,8 @@ #endif struct Place; +using JEventQueue = JMailbox; + class JArrow { friend class JScheduler; @@ -29,6 +31,7 @@ class JArrow { bool m_is_source; // Whether or not this arrow should activate/drain the topology bool m_is_sink; // Whether or not tnis arrow contributes to the final event count JArrowMetrics m_metrics; // Performance information accumulated over all workers + int m_next_place_index=0; std::vector m_listeners; // Downstream Arrows @@ -63,6 +66,9 @@ class JArrow { virtual void initialize() { }; virtual void execute(JArrowMetrics& result, size_t location_id) = 0; +/* + virtual void execute(JEvent* input_event, int input_place_index, size_t* output_event_count, JEvent** output_events, int* output_place_indices, int* next_input_place_index) const; + */ virtual void finalize() {}; diff --git a/src/libraries/JANA/Topology/JEventMapArrow.cc b/src/libraries/JANA/Topology/JEventMapArrow.cc index 421de9408..76a84aadd 100644 --- a/src/libraries/JANA/Topology/JEventMapArrow.cc +++ b/src/libraries/JANA/Topology/JEventMapArrow.cc @@ -73,4 +73,13 @@ void JEventMapArrow::finalize() { } LOG_DEBUG(m_logger) << "Finalized arrow " << get_name() << LOG_END; } - + +/* +void JEventMapArrow::execute(JEvent* input_event, int input_place_index, size_t* output_event_count, JEvent** output_events, int* output_place_indices, int* next_input_place_index) const { + assert(input_place_index == 0); + *output_event_count = 1; + output_events[0] = input_event; + output_place_indices[0] = 1; + *next_input_place_index = 0; +} +*/ From 593fd81d7925c66e031d101cf413493dc67b847f Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 25 Oct 2024 13:17:46 -0400 Subject: [PATCH 06/22] Remove dead JJunctionArrow --- src/libraries/JANA/Topology/JJunctionArrow.h | 101 ---------------- src/programs/unit_tests/CMakeLists.txt | 1 - .../unit_tests/Topology/ArrowTests.cc | 108 ------------------ 3 files changed, 210 deletions(-) delete mode 100644 src/libraries/JANA/Topology/JJunctionArrow.h delete mode 100644 src/programs/unit_tests/Topology/ArrowTests.cc diff --git a/src/libraries/JANA/Topology/JJunctionArrow.h b/src/libraries/JANA/Topology/JJunctionArrow.h deleted file mode 100644 index be8ca8292..000000000 --- a/src/libraries/JANA/Topology/JJunctionArrow.h +++ /dev/null @@ -1,101 +0,0 @@ -// Copyright 2023, Jefferson Science Associates, LLC. -// Subject to the terms in the LICENSE file found in the top-level directory. - -#pragma once - -#include -#include -#include - - - -template -class JJunctionArrow : public JArrow { - -protected: - Place first_input {this, true, 1, 1}; - Place first_output {this, false, 1, 1}; - Place second_input {this, true, 1, 1}; - Place second_output {this, false, 1, 1}; - -public: - using Status = JArrowMetrics::Status; - - JJunctionArrow(std::string name, - bool is_parallel, - bool is_source, - bool is_sink - ) - : JArrow(std::move(name), is_parallel, is_source, is_sink) - { - } - - bool try_pull_all(Data& fi, Data& fo, Data& si, Data& so) { - - bool success; - success = first_input.pull(fi); - if (! success) { - return false; - } - success = first_output.pull(fo); - if (! success) { - first_input.revert(fi); - return false; - } - success = second_input.pull(si); - if (! success) { - first_input.revert(fi); - first_output.revert(fo); - return false; - } - success = second_output.pull(so); - if (! success) { - first_input.revert(fi); - first_output.revert(fo); - second_input.revert(si); - return false; - } - return true; - } - - size_t push_all(Data& fi, Data& fo, Data& si, Data& so) { - size_t message_count = 0; - message_count += first_input.push(fi); - message_count += first_output.push(fo); - message_count += second_input.push(si); - message_count += second_output.push(so); - return message_count; - } - - void execute(JArrowMetrics& result, size_t location_id) final { - - auto start_total_time = std::chrono::steady_clock::now(); - - Data first_input_data {location_id}; - Data first_output_data {location_id}; - Data second_input_data {location_id}; - Data second_output_data {location_id}; - - bool success = try_pull_all(first_input_data, first_output_data, second_input_data, second_output_data); - if (success) { - - auto start_processing_time = std::chrono::steady_clock::now(); - auto process_status = static_cast(this)->process(first_input_data, first_output_data, second_input_data, second_output_data); - auto end_processing_time = std::chrono::steady_clock::now(); - size_t events_processed = push_all(first_input_data, first_output_data, second_input_data, second_output_data); - - auto end_total_time = std::chrono::steady_clock::now(); - auto latency = (end_processing_time - start_processing_time); - auto overhead = (end_total_time - start_total_time) - latency; - result.update(process_status, events_processed, 1, latency, overhead); - return; - } - else { - auto end_total_time = std::chrono::steady_clock::now(); - result.update(JArrowMetrics::Status::ComeBackLater, 0, 1, std::chrono::milliseconds(0), end_total_time - start_total_time); - return; - } - } -}; - - diff --git a/src/programs/unit_tests/CMakeLists.txt b/src/programs/unit_tests/CMakeLists.txt index adcc18650..60fe13339 100644 --- a/src/programs/unit_tests/CMakeLists.txt +++ b/src/programs/unit_tests/CMakeLists.txt @@ -1,7 +1,6 @@ set(TEST_SOURCES - Topology/ArrowTests.cc Topology/JPoolTests.cc Topology/MultiLevelTopologyTests.cc Topology/QueueTests.cc diff --git a/src/programs/unit_tests/Topology/ArrowTests.cc b/src/programs/unit_tests/Topology/ArrowTests.cc deleted file mode 100644 index 2ec71ac7d..000000000 --- a/src/programs/unit_tests/Topology/ArrowTests.cc +++ /dev/null @@ -1,108 +0,0 @@ - -#include -#include -#include - -namespace jana { -namespace arrowtests { - - -struct ArrowTestData { - int x; - double y; -}; - -struct TestJunctionArrow : public JJunctionArrow { - - TestJunctionArrow(JMailbox* qi, - JEventPool* pi, - JEventPool* pd, - JMailbox* qd) - : JJunctionArrow("testjunctionarrow", false, false, true) { - - first_input.set_queue(qi); - first_output.set_pool(pi); - second_input.set_pool(pd); - second_output.set_queue(qd); - } - - Status process(Data& input_int, - Data& output_int, - Data& input_double, - Data& output_double) { - std::cout << "Hello from process" << std::endl; - - REQUIRE(input_int.item_count == 1); - REQUIRE(input_int.reserve_count == 0); // Would be 1 - REQUIRE(output_int.item_count == 0); - REQUIRE(output_int.reserve_count == 0); - REQUIRE(input_double.item_count == 1); - REQUIRE(input_double.reserve_count == 0); - REQUIRE(output_double.item_count == 0); - REQUIRE(output_double.reserve_count == 0); // Would be 1 - - JEvent* x_event = input_int.items[0]; - input_int.items[0] = nullptr; - input_int.item_count = 0; - - // TODO: Maybe user shouldn't be allowed to modify reserve_count at all - // TODO Maybe user should only be allowed to push and pull from ... range...? - - JEvent* y_event = input_double.items[0]; - input_double.items[0] = nullptr; - input_double.item_count = 0; - - auto data = x_event->Get(); - int x = data.at(0)->x; - // Do something useful here - double y = x + 22.2; - y_event->Insert(new ArrowTestData{.x = x, .y = y}); - - output_int.items[0] = x_event; - output_int.item_count = 1; - - output_double.items[0] = y_event; - output_double.item_count = 1; - return Status::KeepGoing; - } - -}; - - -TEST_CASE("ArrowTests_Basic") { - - JApplication app; - app.Initialize(); - auto jcm = app.GetService(); - - JMailbox qi {2, 1, false}; - JEventPool pi {jcm, 5, 1}; - JEventPool pd {jcm, 5, 1}; - JMailbox qd {2, 1, false}; - - TestJunctionArrow a {&qi, &pi, &pd, &qd}; - - JEvent* x = nullptr; - pi.pop(&x, 1, 1, 0); - REQUIRE(x != nullptr); - REQUIRE(x->GetEventNumber() == 0); - x->Insert(new ArrowTestData {.x = 100, .y=0}); - - qi.push_and_unreserve(&x, 1, 0, 0); - JArrowMetrics m; - a.execute(m, 0); - - JEvent* y; - qd.pop_and_reserve(&y, 1, 1, 0); - auto data = y->Get(); - REQUIRE(data.at(0)->y == 122.2); - -} - - -} // namespace arrowtests -} // namespace jana - - - - From 97ade2dc60cbd1bcf4588787d91291430d027f0c Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 25 Oct 2024 13:43:49 -0400 Subject: [PATCH 07/22] Remove queue reservations --- src/libraries/JANA/Topology/JArrow.h | 31 ++----- src/libraries/JANA/Topology/JMailbox.h | 86 ++----------------- src/libraries/JANA/Topology/JSubeventArrow.h | 15 ++-- .../unit_tests/Topology/QueueTests.cc | 6 +- 4 files changed, 21 insertions(+), 117 deletions(-) diff --git a/src/libraries/JANA/Topology/JArrow.h b/src/libraries/JANA/Topology/JArrow.h index 461fbe2db..89e30b3b5 100644 --- a/src/libraries/JANA/Topology/JArrow.h +++ b/src/libraries/JANA/Topology/JArrow.h @@ -66,9 +66,8 @@ class JArrow { virtual void initialize() { }; virtual void execute(JArrowMetrics& result, size_t location_id) = 0; -/* - virtual void execute(JEvent* input_event, int input_place_index, size_t* output_event_count, JEvent** output_events, int* output_place_indices, int* next_input_place_index) const; - */ + + virtual void execute(JEvent* /*input_event*/, int /*current_input_port*/, int& /*next_input_port*/, std::vector>& /*outputs*/) {}; virtual void finalize() {}; @@ -89,7 +88,6 @@ class JArrow { struct Data { std::array items; size_t item_count = 0; - size_t reserve_count = 0; size_t location_id; Data(size_t location_id = 0) : location_id(location_id) { @@ -139,30 +137,17 @@ struct Place { if (is_queue) { auto queue = static_cast*>(place_ref); data.item_count = queue->pop(data.items.data(), min_item_count, max_item_count, data.location_id); - //data.item_count = queue->pop_and_reserve(data.items.data(), min_item_count, max_item_count, data.location_id); return (data.item_count >= min_item_count); } else { auto pool = static_cast(place_ref); data.item_count = pool->pop(data.items.data(), min_item_count, max_item_count, data.location_id); - data.reserve_count = 0; return (data.item_count >= min_item_count); } } else { - if (is_queue) { - // Reserve a space on the output queue - data.item_count = 0; - //auto queue = static_cast*>(place_ref); - data.reserve_count = 0; //queue->reserve(min_item_count, max_item_count, data.location_id); - return true; //(data.reserve_count >= min_item_count); - } - else { - // No need to reserve on pool -- either there is space or limit_events_in_flight=false - data.item_count = 0; - data.reserve_count = 0; - return true; - } + data.item_count = 0; + return true; } } @@ -170,8 +155,7 @@ struct Place { assert(place_ref != nullptr); if (is_queue) { auto queue = static_cast*>(place_ref); - assert(data.reserve_count == 0); - queue->push_and_unreserve(data.items.data(), data.item_count, data.reserve_count, data.location_id); + queue->push(data.items.data(), data.item_count, data.location_id); } else { if (is_input) { @@ -185,17 +169,14 @@ struct Place { assert(place_ref != nullptr); if (is_queue) { auto queue = static_cast*>(place_ref); - assert(data.reserve_count == 0); - queue->push_and_unreserve(data.items.data(), data.item_count, data.reserve_count, data.location_id); + queue->push(data.items.data(), data.item_count, data.location_id); data.item_count = 0; - data.reserve_count = 0; return is_input ? 0 : data.item_count; } else { auto pool = static_cast(place_ref); pool->push(data.items.data(), data.item_count, !is_input, data.location_id); data.item_count = 0; - data.reserve_count = 0; return 1; } } diff --git a/src/libraries/JANA/Topology/JMailbox.h b/src/libraries/JANA/Topology/JMailbox.h index 49a5a8384..8e0e1405d 100644 --- a/src/libraries/JANA/Topology/JMailbox.h +++ b/src/libraries/JANA/Topology/JMailbox.h @@ -14,7 +14,6 @@ /// - pushes and pops return a Status enum, handling the problem of .size() always being stale /// - pops may fail but pushes may not /// - pushes and pops are chunked, reducing contention and handling the failure case cleanly -/// - when the .reserve() method is used, the queue size is bounded /// - the underlying queue may be shared by all threads, NUMA-domain-local, or thread-local /// - the Arrow doesn't have to know anything about locality. /// @@ -58,7 +57,6 @@ class JMailbox : public JQueue { struct LocalQueue { std::mutex mutex; std::deque queue; - size_t reserved_count = 0; }; // TODO: Copy these params into DLMB for better locality @@ -114,35 +112,11 @@ class JMailbox : public JQueue { return m_queues[location_id].queue.size(); } - /// reserve(requested_count) keeps our queues bounded in size. The caller should - /// reserve their desired chunk size on the output queue first. The output - /// queue will return a reservation which is less than or equal to requested_count. - /// The caller may then request as many items from the input queue as have been - /// reserved on the output queue. Note that because the input queue may return - /// fewer items than requested, the caller must push their original reserved_count - /// alongside the items, to avoid a "reservation leak". - size_t reserve(size_t requested_count, size_t location_id = 0) { - - LocalQueue& mb = m_queues[location_id]; - std::lock_guard lock(mb.mutex); - size_t doable_count = m_capacity - mb.queue.size() - mb.reserved_count; - if (doable_count > 0) { - size_t reservation = std::min(doable_count, requested_count); - mb.reserved_count += reservation; - return reservation; - } - return 0; - }; - - /// push(items, reserved_count, location_id) This function will always - /// succeed, although it may exceed the threshold if the caller didn't reserve - /// space, and it may take a long time because it will wait on a mutex. - /// Note that if the caller had called reserve(), they must pass in the reserved_count here. - Status push(std::vector& buffer, size_t reserved_count = 0, size_t location_id = 0) { + /// push(items, location_id) This function will always succeed. + Status push(std::vector& buffer, size_t location_id = 0) { auto& mb = m_queues[location_id]; std::lock_guard lock(mb.mutex); - mb.reserved_count -= reserved_count; for (const T& t : buffer) { mb.queue.push_back(std::move(t)); } @@ -207,9 +181,6 @@ class JMailbox : public JQueue { return Status::Empty; } - - - bool try_push(T* buffer, size_t count, size_t location_id = 0) { auto& mb = m_queues[location_id]; std::lock_guard lock(mb.mutex); @@ -221,13 +192,11 @@ class JMailbox : public JQueue { return true; } - void push_and_unreserve(T* buffer, size_t count, size_t reserved_count = 0, size_t location_id = 0) { + void push(T* buffer, size_t count, size_t location_id = 0) { auto& mb = m_queues[location_id]; std::lock_guard lock(mb.mutex); - assert(reserved_count <= mb.reserved_count); assert(mb.queue.size() + count <= m_capacity); - mb.reserved_count -= reserved_count; for (size_t i=0; i lock(mb.mutex); - - if (mb.queue.size() < min_requested_count) return 0; - - auto nitems = std::min(max_requested_count, mb.queue.size()); - mb.reserved_count += nitems; - - for (size_t i=0; i lock(mb.mutex); - size_t available_count = m_capacity - mb.queue.size() - mb.reserved_count; - size_t count = std::min(available_count, max_requested_count); - if (count < min_requested_count) { - return 0; - } - mb.reserved_count += count; - return count; - }; - - void unreserve(size_t reserved_count, size_t location_id) { - - LocalQueue& mb = m_queues[location_id]; - std::lock_guard lock(mb.mutex); - assert(reserved_count <= mb.reserved_count); - mb.reserved_count -= reserved_count; - }; - }; template <> -inline void JMailbox*>::push_and_unreserve(std::shared_ptr** buffer, size_t count, size_t reserved_count, size_t location_id) { +inline void JMailbox::push(JEvent** buffer, size_t count, size_t location_id) { auto& mb = m_queues[location_id]; std::lock_guard lock(mb.mutex); - assert(reserved_count <= mb.reserved_count); assert(mb.queue.size() + count <= m_capacity); - mb.reserved_count -= reserved_count; for (size_t i=0; iget()->GetEventNumber() << LOG_END; + LOG_TRACE(m_logger) << "JMailbox: push_and_unreserve(): queue #" << m_id << ", event #" << buffer[i]->GetEventNumber() << LOG_END; mb.queue.push_back(buffer[i]); buffer[i] = nullptr; } } template <> -inline size_t JMailbox*>::pop_and_reserve(std::shared_ptr** buffer, size_t min_requested_count, size_t max_requested_count, size_t location_id) { +inline size_t JMailbox::pop(JEvent** buffer, size_t min_requested_count, size_t max_requested_count, size_t location_id) { auto& mb = m_queues[location_id]; std::lock_guard lock(mb.mutex); @@ -314,11 +243,10 @@ inline size_t JMailbox*>::pop_and_reserve(std::shared_pt if (mb.queue.size() < min_requested_count) return 0; auto nitems = std::min(max_requested_count, mb.queue.size()); - mb.reserved_count += nitems; for (size_t i=0; iget()->GetEventNumber() << LOG_END; + LOG_TRACE(m_logger) << "JMailbox: pop(): queue #" << m_id << ", event #" << buffer[i]->GetEventNumber() << LOG_END; mb.queue.pop_front(); } return nitems; diff --git a/src/libraries/JANA/Topology/JSubeventArrow.h b/src/libraries/JANA/Topology/JSubeventArrow.h index 8380089f1..a51ba63d6 100644 --- a/src/libraries/JANA/Topology/JSubeventArrow.h +++ b/src/libraries/JANA/Topology/JSubeventArrow.h @@ -111,9 +111,7 @@ void JSplitArrow::execute(JArrowMetrics& result, size_t locatio JEvent* event = nullptr; bool success; - size_t reserved_size = m_outbox->reserve(1); - size_t actual_size = reserved_size; - // TODO: Exit early if we don't have enough space on output queue + size_t actual_size = 1; auto in_status = m_inbox->pop(event, success, location_id); auto start_latency_time = std::chrono::steady_clock::now(); @@ -137,7 +135,7 @@ void JSplitArrow::execute(JArrowMetrics& result, size_t locatio size_t output_size = wrapped.size(); if (success) { assert(m_outbox != nullptr); - out_status = m_outbox->push(wrapped, reserved_size, location_id); + out_status = m_outbox->push(wrapped, location_id); } auto end_queue_time = std::chrono::steady_clock::now(); @@ -160,8 +158,7 @@ void JSubeventArrow::execute(JArrowMetrics& result, size_t loca // TODO: Think more carefully about subevent bucket size std::vector> inputs; - size_t downstream_accepts = m_outbox->reserve(1, location_id); - auto in_status = m_inbox->pop(inputs, downstream_accepts, location_id); + auto in_status = m_inbox->pop(inputs, 1, location_id); auto start_latency_time = std::chrono::steady_clock::now(); std::vector> outputs; @@ -176,7 +173,7 @@ void JSubeventArrow::execute(JArrowMetrics& result, size_t loca if (outputs_size > 0) { assert(m_outbox != nullptr); - out_status = m_outbox->push(outputs, downstream_accepts, location_id); + out_status = m_outbox->push(outputs, location_id); } auto end_queue_time = std::chrono::steady_clock::now(); @@ -201,7 +198,7 @@ void JMergeArrow::execute(JArrowMetrics& result, size_t locatio // TODO: Think more carefully about subevent bucket size std::vector> inputs; - size_t downstream_accepts = m_outbox->reserve(1, location_id); + size_t downstream_accepts = true; auto in_status = m_inbox->pop(inputs, downstream_accepts, location_id); auto start_latency_time = std::chrono::steady_clock::now(); @@ -240,7 +237,7 @@ void JMergeArrow::execute(JArrowMetrics& result, size_t locatio auto end_latency_time = std::chrono::steady_clock::now(); auto outputs_size = outputs.size(); - auto out_status = m_outbox->push(outputs, downstream_accepts, location_id); + auto out_status = m_outbox->push(outputs, location_id); auto end_queue_time = std::chrono::steady_clock::now(); diff --git a/src/programs/unit_tests/Topology/QueueTests.cc b/src/programs/unit_tests/Topology/QueueTests.cc index 810aeb95a..a6641b2d9 100644 --- a/src/programs/unit_tests/Topology/QueueTests.cc +++ b/src/programs/unit_tests/Topology/QueueTests.cc @@ -27,13 +27,11 @@ TEST_CASE("QueueTests_Basic") { items[1] = new int {44}; items[2] = new int {55}; - size_t reserve_count = q.reserve(3, 5, 0); - REQUIRE(reserve_count == 5); - q.push_and_unreserve(items, 3, reserve_count, 0); + q.push(items, 3, 0); REQUIRE(q.size() == 3); - count = q.pop_and_reserve(items, 2, 2, 0); + count = q.pop(items, 2, 2, 0); REQUIRE(count == 2); REQUIRE(q.size() == 1); } From 9320920eafc16c9971d53c535ec2ef9028c53606 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 25 Oct 2024 17:08:09 -0400 Subject: [PATCH 08/22] Simplest example of new triggered arrow concept --- src/libraries/JANA/CMakeLists.txt | 1 + src/libraries/JANA/Topology/JArrow.cc | 57 +++++++++++ src/libraries/JANA/Topology/JArrow.h | 27 ++++- src/libraries/JANA/Topology/JTriggeredArrow.h | 47 +++++++++ src/programs/unit_tests/CMakeLists.txt | 1 + .../unit_tests/Topology/JArrowTests.cc | 98 +++++++++++++++++++ 6 files changed, 227 insertions(+), 4 deletions(-) create mode 100644 src/libraries/JANA/Topology/JArrow.cc create mode 100644 src/libraries/JANA/Topology/JTriggeredArrow.h create mode 100644 src/programs/unit_tests/Topology/JArrowTests.cc diff --git a/src/libraries/JANA/CMakeLists.txt b/src/libraries/JANA/CMakeLists.txt index 396b035f3..e0b181f85 100644 --- a/src/libraries/JANA/CMakeLists.txt +++ b/src/libraries/JANA/CMakeLists.txt @@ -19,6 +19,7 @@ set(JANA2_SOURCES Engine/JPerfMetrics.cc Engine/JPerfSummary.cc + Topology/JArrow.cc Topology/JEventSourceArrow.cc Topology/JEventMapArrow.cc Topology/JEventTapArrow.cc diff --git a/src/libraries/JANA/Topology/JArrow.cc b/src/libraries/JANA/Topology/JArrow.cc new file mode 100644 index 000000000..18c8656f3 --- /dev/null +++ b/src/libraries/JANA/Topology/JArrow.cc @@ -0,0 +1,57 @@ + +#include + + +void JArrow::attach(JMailbox* queue, size_t port) { + // Place index is relative to whether it is an input or not + // Port index, however, is agnostic to whether it is an input or not + if (port >= m_places.size()) { + throw JException("Attempting to attach to a non-existent place! arrow=%s, port=%d", m_name.c_str(), port); + } + m_places[port]->is_queue = true; + m_places[port]->place_ref = queue; +} + + +void JArrow::attach(JEventPool* pool, size_t port) { + // Place index is relative to whether it is an input or not + // Port index, however, is agnostic to whether it is an input or not + if (port >= m_places.size()) { + throw JException("Attempting to attach to a non-existent place! arrow=%s, port=%d", m_name.c_str(), port); + } + m_places[port]->is_queue = false; + m_places[port]->place_ref = pool; +} + + +JEvent* JArrow::pull(size_t input_port, size_t location_id) { + JEvent* event = nullptr; + auto& place = m_places[input_port]; + if (place->is_queue) { + auto queue = static_cast*>(place->place_ref); + queue->pop(&event, 1, 1, location_id); + } + else { + auto pool = static_cast(place->place_ref); + pool->pop(&event, 1, 1, location_id); + } + // If ether pop() failed, the returned event is nullptr + return event; +} + + +void JArrow::push(OutputData& outputs, size_t output_count, size_t location_id) { + for (size_t output = 0; output < output_count; ++output) { + JEvent* event = outputs[output].first; + int port = outputs[output].second; + if (m_places[port]->is_queue) { + auto queue = static_cast*>(m_places[port]->place_ref); + queue->push(&event, 1, location_id); + } + else { + auto pool = static_cast(m_places[port]->place_ref); + bool clear_event = !m_places[port]->is_input; + pool->push(&event, 1, clear_event, location_id); + } + } +} diff --git a/src/libraries/JANA/Topology/JArrow.h b/src/libraries/JANA/Topology/JArrow.h index 89e30b3b5..bab1ab164 100644 --- a/src/libraries/JANA/Topology/JArrow.h +++ b/src/libraries/JANA/Topology/JArrow.h @@ -25,20 +25,22 @@ class JArrow { friend class JScheduler; friend class JTopologyBuilder; +public: + using OutputData = std::array, 2>; + private: std::string m_name; // Used for human understanding bool m_is_parallel; // Whether or not it is safe to parallelize bool m_is_source; // Whether or not this arrow should activate/drain the topology bool m_is_sink; // Whether or not tnis arrow contributes to the final event count JArrowMetrics m_metrics; // Performance information accumulated over all workers - int m_next_place_index=0; std::vector m_listeners; // Downstream Arrows protected: - // This is usable by subclasses. JLogger m_logger; std::vector m_places; // Will eventually supplant m_listeners + int m_next_input_port=0; public: std::string get_name() { return m_name; } @@ -55,6 +57,12 @@ class JArrow { void set_is_sink(bool is_sink) { m_is_sink = is_sink; } + JArrow() { + m_is_parallel = false; + m_is_source = false; + m_is_sink = false; + } + JArrow(std::string name, bool is_parallel, bool is_source, bool is_sink) : m_name(std::move(name)), m_is_parallel(is_parallel), m_is_source(is_source), m_is_sink(is_sink) { @@ -66,8 +74,6 @@ class JArrow { virtual void initialize() { }; virtual void execute(JArrowMetrics& result, size_t location_id) = 0; - - virtual void execute(JEvent* /*input_event*/, int /*current_input_port*/, int& /*next_input_port*/, std::vector>& /*outputs*/) {}; virtual void finalize() {}; @@ -83,6 +89,12 @@ class JArrow { m_places.push_back(place); } }; + + void attach(JMailbox* queue, size_t port); + void attach(JEventPool* pool, size_t port); + + JEvent* pull(size_t input_port, size_t location_id); + void push(OutputData& outputs, size_t output_count, size_t location_id); }; struct Data { @@ -95,6 +107,7 @@ struct Data { } }; + struct Place { void* place_ref = nullptr; bool is_queue = true; @@ -102,6 +115,12 @@ struct Place { size_t min_item_count = 1; size_t max_item_count = 1; + Place(JArrow* parent, bool is_input) { + assert(parent != nullptr); + parent->attach(this); + this->is_input = is_input; + } + Place(JArrow* parent, bool is_input, size_t min_item_count, size_t max_item_count) { assert(parent != nullptr); parent->attach(this); diff --git a/src/libraries/JANA/Topology/JTriggeredArrow.h b/src/libraries/JANA/Topology/JTriggeredArrow.h new file mode 100644 index 000000000..8bdc8eded --- /dev/null +++ b/src/libraries/JANA/Topology/JTriggeredArrow.h @@ -0,0 +1,47 @@ + +#pragma once + +#include + +template +struct JTriggeredArrow : public JArrow { + + + void execute(JArrowMetrics& result, size_t location_id) final { + + auto start_total_time = std::chrono::steady_clock::now(); + + JEvent* input = pull(m_next_input_port, location_id); + + if (input == nullptr) { + // Failed to obtain the input we needed; arrow is NOT ready to fire + + auto end_total_time = std::chrono::steady_clock::now(); + + result.update(JArrowMetrics::Status::ComeBackLater, 0, 1, std::chrono::milliseconds(0), end_total_time - start_total_time); + } + else { + // Obtained the input we needed; arrow is ready to fire + + auto start_processing_time = std::chrono::steady_clock::now(); + + OutputData outputs; + size_t output_count; + JArrowMetrics::Status process_status = JArrowMetrics::Status::KeepGoing; + + static_cast(this)->fire(input, outputs, output_count, process_status); + + auto end_processing_time = std::chrono::steady_clock::now(); + + push(outputs, output_count, location_id); + + auto end_total_time = std::chrono::steady_clock::now(); + + auto latency = (end_processing_time - start_processing_time); + auto overhead = (end_total_time - start_total_time) - latency; + result.update(process_status, true, 1, latency, overhead); + } + } +}; + + diff --git a/src/programs/unit_tests/CMakeLists.txt b/src/programs/unit_tests/CMakeLists.txt index 60fe13339..2bddf82af 100644 --- a/src/programs/unit_tests/CMakeLists.txt +++ b/src/programs/unit_tests/CMakeLists.txt @@ -1,6 +1,7 @@ set(TEST_SOURCES + Topology/JArrowTests.cc Topology/JPoolTests.cc Topology/MultiLevelTopologyTests.cc Topology/QueueTests.cc diff --git a/src/programs/unit_tests/Topology/JArrowTests.cc b/src/programs/unit_tests/Topology/JArrowTests.cc new file mode 100644 index 000000000..6ea867112 --- /dev/null +++ b/src/programs/unit_tests/Topology/JArrowTests.cc @@ -0,0 +1,98 @@ + +#include +#include + +namespace jana::topology::jarrowtests { + +struct TestData { int x; }; + +struct BasicParallelArrow : public JTriggeredArrow { + + Place m_input {this, true }; + Place m_output {this, false }; + + BasicParallelArrow() { + set_is_parallel(true); + } + + void fire(JEvent* input, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& process_status) { + + input->Insert(new TestData {.x=22}); + + outputs[0] = {input, 1}; + output_count = 1; + process_status = JArrowMetrics::Status::KeepGoing; + + } +}; + + +TEST_CASE("BasicParallelArrow_Fire") { + + JApplication app; + app.Initialize(); + auto event = std::make_shared(&app); + + BasicParallelArrow sut; + JArrow::OutputData outputs; + size_t output_count; + JArrowMetrics::Status status; + + sut.fire(event.get(), outputs, output_count, status); + + REQUIRE(event->GetSingle()->x == 22); + REQUIRE(output_count == 1); + REQUIRE(outputs[0].first == event.get()); + REQUIRE(outputs[0].second == 1); + REQUIRE(status == JArrowMetrics::Status::KeepGoing); +} + + + +TEST_CASE("BasicParallelArrow_ExecuteSucceeds") { + + JApplication app; + app.Initialize(); + + BasicParallelArrow sut; + JEventPool input_pool(app.GetService(), 10, 1); + JMailbox output_queue(10); + + sut.attach(&input_pool, 0); + sut.attach(&output_queue, 1); + + JArrowMetrics metrics; + sut.execute(metrics, 0); + + REQUIRE(metrics.get_last_status() == JArrowMetrics::Status::KeepGoing); + REQUIRE(metrics.get_total_message_count() == 1); + REQUIRE(output_queue.size() == 1); + JEvent* event; + output_queue.pop(&event, 1, 1, 0); + REQUIRE(event->GetSingle()->x == 22); +} + + +TEST_CASE("BasicParallelArrow_ExecuteFails") { + + JApplication app; + app.Initialize(); + + BasicParallelArrow sut; + JMailbox input_queue(10); + JMailbox output_queue(10); + + sut.attach(&input_queue, 0); + sut.attach(&output_queue, 1); + + JArrowMetrics metrics; + sut.execute(metrics, 0); + + REQUIRE(metrics.get_last_status() == JArrowMetrics::Status::ComeBackLater); + REQUIRE(metrics.get_total_message_count() == 0); + REQUIRE(output_queue.size() == 0); +} + + + +} // namespace jana::topology::jarrowtests From 9debf01ba2fdcc7d06450b816bdf85b649eb6f37 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 25 Oct 2024 18:48:59 -0400 Subject: [PATCH 09/22] Migrate JEventTapArrow --- src/libraries/JANA/Topology/JArrow.h | 2 -- src/libraries/JANA/Topology/JArrowMetrics.h | 23 ++++++++----------- src/libraries/JANA/Topology/JEventTapArrow.cc | 12 ++++++---- src/libraries/JANA/Topology/JEventTapArrow.h | 9 +++++--- .../JANA/Topology/JTopologyBuilder.cc | 4 ++-- src/libraries/JANA/Topology/JTriggeredArrow.h | 2 +- 6 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/libraries/JANA/Topology/JArrow.h b/src/libraries/JANA/Topology/JArrow.h index bab1ab164..579d12ccf 100644 --- a/src/libraries/JANA/Topology/JArrow.h +++ b/src/libraries/JANA/Topology/JArrow.h @@ -65,8 +65,6 @@ class JArrow { JArrow(std::string name, bool is_parallel, bool is_source, bool is_sink) : m_name(std::move(name)), m_is_parallel(is_parallel), m_is_source(is_source), m_is_sink(is_sink) { - - m_metrics.clear(); }; virtual ~JArrow() = default; diff --git a/src/libraries/JANA/Topology/JArrowMetrics.h b/src/libraries/JANA/Topology/JArrowMetrics.h index 12e783d79..04f53bd92 100644 --- a/src/libraries/JANA/Topology/JArrowMetrics.h +++ b/src/libraries/JANA/Topology/JArrowMetrics.h @@ -17,20 +17,15 @@ class JArrowMetrics { mutable std::mutex m_mutex; // Mutex is mutable so that we can lock before reading from a const ref - Status m_last_status; - size_t m_total_message_count; - size_t m_last_message_count; - size_t m_total_queue_visits; - size_t m_last_queue_visits; - duration_t m_total_latency; - duration_t m_last_latency; - duration_t m_total_queue_latency; - duration_t m_last_queue_latency; - - - // TODO: We might want to add a timestamp, so that - // the 'last_*' measurements can reflect the most recent value, - // rather than the last-to-be-accumulated value. + Status m_last_status = Status::NotRunYet; + size_t m_total_message_count = 0; + size_t m_last_message_count = 0; + size_t m_total_queue_visits = 0; + size_t m_last_queue_visits = 0; + duration_t m_total_latency = duration_t::zero(); + duration_t m_last_latency = duration_t::zero(); + duration_t m_total_queue_latency = duration_t::zero(); + duration_t m_last_queue_latency = duration_t::zero(); public: void clear() { diff --git a/src/libraries/JANA/Topology/JEventTapArrow.cc b/src/libraries/JANA/Topology/JEventTapArrow.cc index e66d9b522..67f1b89b1 100644 --- a/src/libraries/JANA/Topology/JEventTapArrow.cc +++ b/src/libraries/JANA/Topology/JEventTapArrow.cc @@ -8,14 +8,15 @@ #include -JEventTapArrow::JEventTapArrow(std::string name) - : JPipelineArrow(std::move(name), false, false, false) {} +JEventTapArrow::JEventTapArrow(std::string name) { + set_name(name); +} void JEventTapArrow::add_processor(JEventProcessor* proc) { m_procs.push_back(proc); } -void JEventTapArrow::process(JEvent* event, bool& success, JArrowMetrics::Status& status) { +void JEventTapArrow::fire(JEvent* event, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status) { LOG_DEBUG(m_logger) << "Executing arrow " << get_name() << " for event# " << event->GetEventNumber() << LOG_END; for (JEventProcessor* proc : m_procs) { @@ -24,9 +25,10 @@ void JEventTapArrow::process(JEvent* event, bool& success, JArrowMetrics::Status proc->DoTap(*event); } } - LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " for event# " << event->GetEventNumber() << LOG_END; - success = true; + outputs[0] = {event, 1}; + output_count = 1; status = JArrowMetrics::Status::KeepGoing; + LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " for event# " << event->GetEventNumber() << LOG_END; } void JEventTapArrow::initialize() { diff --git a/src/libraries/JANA/Topology/JEventTapArrow.h b/src/libraries/JANA/Topology/JEventTapArrow.h index f2020dea6..93f3c125f 100644 --- a/src/libraries/JANA/Topology/JEventTapArrow.h +++ b/src/libraries/JANA/Topology/JEventTapArrow.h @@ -3,22 +3,25 @@ #pragma once -#include +#include "JANA/Topology/JTriggeredArrow.h" class JEventProcessor; class JEvent; -class JEventTapArrow : public JPipelineArrow { +class JEventTapArrow : public JTriggeredArrow { private: std::vector m_procs; + Place m_input {this, true }; + Place m_output {this, false }; public: JEventTapArrow(std::string name); void add_processor(JEventProcessor* proc); - void process(JEvent* event, bool& success, JArrowMetrics::Status& status); + + void fire(JEvent* input, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status); void initialize() final; void finalize() final; }; diff --git a/src/libraries/JANA/Topology/JTopologyBuilder.cc b/src/libraries/JANA/Topology/JTopologyBuilder.cc index 6c9cd790f..c516871c0 100644 --- a/src/libraries/JANA/Topology/JTopologyBuilder.cc +++ b/src/libraries/JANA/Topology/JTopologyBuilder.cc @@ -337,8 +337,8 @@ void JTopologyBuilder::attach_level(JEventLevel current_level, JUnfoldArrow* par tap_arrow = new JEventTapArrow(level_str+"Tap"); for (JEventProcessor* proc : tappable_procs_at_level) { tap_arrow->add_processor(proc); - tap_arrow->set_input(pool_at_level); - tap_arrow->set_output(pool_at_level); + tap_arrow->attach(pool_at_level, 0); + tap_arrow->attach(pool_at_level, 1); } arrows.push_back(tap_arrow); } diff --git a/src/libraries/JANA/Topology/JTriggeredArrow.h b/src/libraries/JANA/Topology/JTriggeredArrow.h index 8bdc8eded..10759096f 100644 --- a/src/libraries/JANA/Topology/JTriggeredArrow.h +++ b/src/libraries/JANA/Topology/JTriggeredArrow.h @@ -39,7 +39,7 @@ struct JTriggeredArrow : public JArrow { auto latency = (end_processing_time - start_processing_time); auto overhead = (end_total_time - start_total_time) - latency; - result.update(process_status, true, 1, latency, overhead); + result.update(process_status, 1, 1, latency, overhead); } } }; From 032764a74c353324991541dfc04cb9c8b06a4bc9 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 25 Oct 2024 20:39:15 -0400 Subject: [PATCH 10/22] Migrate all JPipelineArrows to JTriggeredArrows --- .../SubeventExample/SubeventExample.cc | 8 +- src/libraries/JANA/Topology/JEventMapArrow.cc | 22 ++--- src/libraries/JANA/Topology/JEventMapArrow.h | 9 +- .../JANA/Topology/JEventSourceArrow.h | 2 +- src/libraries/JANA/Topology/JEventTapArrow.h | 2 +- .../JANA/Topology/JTopologyBuilder.cc | 8 +- .../unit_tests/Topology/SubeventTests.cc | 4 +- .../Topology/TestTopologyComponents.h | 89 ++++++++++++------- 8 files changed, 83 insertions(+), 61 deletions(-) diff --git a/src/examples/SubeventExample/SubeventExample.cc b/src/examples/SubeventExample/SubeventExample.cc index 8faf44c7c..d99feccb9 100644 --- a/src/examples/SubeventExample/SubeventExample.cc +++ b/src/examples/SubeventExample/SubeventExample.cc @@ -108,12 +108,12 @@ int main() { auto merge_arrow = new JMergeArrow("merge", &processor, &subevents_out, &events_out); auto source_arrow = new JEventSourceArrow("simpleSource", {source}); - source_arrow->set_input(topology->event_pool); - source_arrow->set_output(&events_in); + source_arrow->attach(topology->event_pool, 0); + source_arrow->attach(&events_in, 1); auto proc_arrow = new JEventMapArrow("simpleProcessor"); - proc_arrow->set_input(&events_out); - proc_arrow->set_output(topology->event_pool); + proc_arrow->attach(&events_out, 0); + proc_arrow->attach(topology->event_pool, 1); proc_arrow->add_processor(new SimpleProcessor); builder.arrows.push_back(source_arrow); diff --git a/src/libraries/JANA/Topology/JEventMapArrow.cc b/src/libraries/JANA/Topology/JEventMapArrow.cc index 76a84aadd..6e971d2a7 100644 --- a/src/libraries/JANA/Topology/JEventMapArrow.cc +++ b/src/libraries/JANA/Topology/JEventMapArrow.cc @@ -10,8 +10,10 @@ #include -JEventMapArrow::JEventMapArrow(std::string name) - : JPipelineArrow(std::move(name), true, false, false) {} +JEventMapArrow::JEventMapArrow(std::string name) { + set_name(name); + set_is_parallel(true); +} void JEventMapArrow::add_source(JEventSource* source) { m_sources.push_back(source); @@ -25,7 +27,7 @@ void JEventMapArrow::add_processor(JEventProcessor* processor) { m_procs.push_back(processor); } -void JEventMapArrow::process(JEvent* event, bool& success, JArrowMetrics::Status& status) { +void JEventMapArrow::fire(JEvent* event, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status) { LOG_DEBUG(m_logger) << "Executing arrow " << get_name() << " for event# " << event->GetEventNumber() << LOG_END; for (JEventSource* source : m_sources) { @@ -46,7 +48,8 @@ void JEventMapArrow::process(JEvent* event, bool& success, JArrowMetrics::Status } } LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " for event# " << event->GetEventNumber() << LOG_END; - success = true; + outputs[0] = {event, 1}; + output_count = 1; status = JArrowMetrics::Status::KeepGoing; } @@ -73,13 +76,4 @@ void JEventMapArrow::finalize() { } LOG_DEBUG(m_logger) << "Finalized arrow " << get_name() << LOG_END; } - -/* -void JEventMapArrow::execute(JEvent* input_event, int input_place_index, size_t* output_event_count, JEvent** output_events, int* output_place_indices, int* next_input_place_index) const { - assert(input_place_index == 0); - *output_event_count = 1; - output_events[0] = input_event; - output_place_indices[0] = 1; - *next_input_place_index = 0; -} -*/ + diff --git a/src/libraries/JANA/Topology/JEventMapArrow.h b/src/libraries/JANA/Topology/JEventMapArrow.h index 563dfe438..e8d9e1230 100644 --- a/src/libraries/JANA/Topology/JEventMapArrow.h +++ b/src/libraries/JANA/Topology/JEventMapArrow.h @@ -3,7 +3,7 @@ #pragma once -#include +#include class JEventPool; class JEventSource; @@ -12,9 +12,12 @@ class JEventProcessor; class JEvent; -class JEventMapArrow : public JPipelineArrow { +class JEventMapArrow : public JTriggeredArrow { private: + Place m_input {this, true }; + Place m_output {this, false }; + std::vector m_sources; std::vector m_unfolders; std::vector m_procs; @@ -26,7 +29,7 @@ class JEventMapArrow : public JPipelineArrow { void add_unfolder(JEventUnfolder* unfolder); void add_processor(JEventProcessor* proc); - void process(JEvent* event, bool& success, JArrowMetrics::Status& status); + void fire(JEvent* input, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status); void initialize() final; void finalize() final; diff --git a/src/libraries/JANA/Topology/JEventSourceArrow.h b/src/libraries/JANA/Topology/JEventSourceArrow.h index 2ee89821d..655881d45 100644 --- a/src/libraries/JANA/Topology/JEventSourceArrow.h +++ b/src/libraries/JANA/Topology/JEventSourceArrow.h @@ -3,7 +3,7 @@ // Subject to the terms in the LICENSE file found in the top-level directory. #pragma once -#include +#include class JEventSourceArrow : public JArrow { diff --git a/src/libraries/JANA/Topology/JEventTapArrow.h b/src/libraries/JANA/Topology/JEventTapArrow.h index 93f3c125f..ec5265b61 100644 --- a/src/libraries/JANA/Topology/JEventTapArrow.h +++ b/src/libraries/JANA/Topology/JEventTapArrow.h @@ -21,7 +21,7 @@ class JEventTapArrow : public JTriggeredArrow { void add_processor(JEventProcessor* proc); - void fire(JEvent* input, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status); + void fire(JEvent* event, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status); void initialize() final; void finalize() final; }; diff --git a/src/libraries/JANA/Topology/JTopologyBuilder.cc b/src/libraries/JANA/Topology/JTopologyBuilder.cc index c516871c0..29fa12c6c 100644 --- a/src/libraries/JANA/Topology/JTopologyBuilder.cc +++ b/src/libraries/JANA/Topology/JTopologyBuilder.cc @@ -287,8 +287,8 @@ void JTopologyBuilder::attach_level(JEventLevel current_level, JUnfoldArrow* par for (JEventUnfolder* unf: unfolders_at_level) { map1_arrow->add_unfolder(unf); } - map1_arrow->set_input(pool_at_level); - map1_arrow->set_output(pool_at_level); + map1_arrow->attach(pool_at_level, 0); + map1_arrow->attach(pool_at_level, 1); arrows.push_back(map1_arrow); } @@ -322,8 +322,8 @@ void JTopologyBuilder::attach_level(JEventLevel current_level, JUnfoldArrow* par map2_arrow = new JEventMapArrow(level_str+"Map2"); for (JEventProcessor* proc : mappable_procs_at_level) { map2_arrow->add_processor(proc); - map2_arrow->set_input(pool_at_level); - map2_arrow->set_output(pool_at_level); + map2_arrow->attach(pool_at_level, 0); + map2_arrow->attach(pool_at_level, 1); } arrows.push_back(map2_arrow); } diff --git a/src/programs/unit_tests/Topology/SubeventTests.cc b/src/programs/unit_tests/Topology/SubeventTests.cc index c35159b20..4883107e3 100644 --- a/src/programs/unit_tests/Topology/SubeventTests.cc +++ b/src/programs/unit_tests/Topology/SubeventTests.cc @@ -107,8 +107,8 @@ TEST_CASE("Basic subevent arrow functionality") { source_arrow->set_output(&events_in); auto proc_arrow = new JEventMapArrow("simpleProcessor"); - proc_arrow->set_input(&events_out); - proc_arrow->set_output(topology.event_pool); + proc_arrow->attach(&events_out, 0); + proc_arrow->attach(topology.event_pool, 1); proc_arrow->add_processor(new SimpleProcessor); topology.arrows.push_back(source_arrow); diff --git a/src/programs/unit_tests/Topology/TestTopologyComponents.h b/src/programs/unit_tests/Topology/TestTopologyComponents.h index b9a6e6bcb..0bf2b8734 100644 --- a/src/programs/unit_tests/Topology/TestTopologyComponents.h +++ b/src/programs/unit_tests/Topology/TestTopologyComponents.h @@ -5,7 +5,7 @@ #pragma once #include "JANA/Topology/JArrowMetrics.h" -#include +#include "JANA/Topology/JTriggeredArrow.h" #include struct EventData { @@ -14,93 +14,118 @@ struct EventData { double z = 0.0; }; -struct RandIntArrow : public JPipelineArrow { +struct RandIntArrow : public JTriggeredArrow { + + Place m_input {this, true }; + Place m_output {this, false }; size_t emit_limit = 20; // How many to emit size_t emit_count = 0; // How many emitted so far int emit_sum = 0; // Sum of all ints emitted so far - RandIntArrow(std::string name, JEventPool* pool, JMailbox* output_queue) - : JPipelineArrow(name, false, true, false) { - this->set_input(pool); - this->set_output(output_queue); + RandIntArrow(std::string name, JEventPool* pool, JMailbox* output_queue) { + set_name(name); + set_is_source(true); + attach(pool, 0); + attach(output_queue, 1); } - void process(JEvent* event, bool& success, JArrowMetrics::Status& status) { + void fire(JEvent* event, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status) { if (emit_count >= emit_limit) { - success = false; + outputs[0] = {event, 0}; // Send event back to the input pool + output_count = 1; status = JArrowMetrics::Status::Finished; return; } auto data = new EventData {7}; - event->Insert(data, "first"); emit_sum += data->x; emit_count += 1; - LOG_DEBUG(JArrow::m_logger) << "RandIntSource emitted event " << emit_count << " with value " << data->x << LOG_END; - success = true; + + outputs[0] = {event, 1}; // Send event back to the input pool + output_count = 1; status = (emit_count == emit_limit) ? JArrowMetrics::Status::Finished : JArrowMetrics::Status::KeepGoing; // This design lets us declare Finished immediately on the last event, instead of after + + LOG_DEBUG(JArrow::m_logger) << "RandIntSource emitted event " << emit_count << " with value " << data->x << LOG_END; } }; -struct MultByTwoArrow : public JPipelineArrow { +struct MultByTwoArrow : public JTriggeredArrow { - MultByTwoArrow(std::string name, JMailbox* input_queue, JMailbox* output_queue) - : JPipelineArrow(name, true, false, false) { - this->set_input(input_queue); - this->set_output(output_queue); + Place m_input {this, true }; + Place m_output {this, false }; + + MultByTwoArrow(std::string name, JMailbox* input_queue, JMailbox* output_queue) { + set_name(name); + set_is_parallel(true); + attach(input_queue, 0); + attach(output_queue, 1); } - void process(JEvent* event, bool& success, JArrowMetrics::Status& status) { + void fire(JEvent* event, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status) { auto prev = event->Get("first"); auto x = prev.at(0)->x; auto next = new EventData { .x=x, .y=x*2.0 }; event->Insert(next, "second"); - success = true; + + outputs[0] = {event, 1}; + output_count = 1; status = JArrowMetrics::Status::KeepGoing; } }; -struct SubOneArrow : public JPipelineArrow { +struct SubOneArrow : public JTriggeredArrow { + + Place m_input {this, true }; + Place m_output {this, false }; - SubOneArrow(std::string name, JMailbox* input_queue, JMailbox* output_queue) - : JPipelineArrow(name, true, false, false) { - this->set_input(input_queue); - this->set_output(output_queue); + SubOneArrow(std::string name, JMailbox* input_queue, JMailbox* output_queue) { + set_name(name); + set_is_parallel(true); + attach(input_queue, 0); + attach(output_queue, 1); } - void process(JEvent* event, bool& success, JArrowMetrics::Status& status) { + void fire(JEvent* event, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status) { auto prev = event->Get("second"); auto x = prev.at(0)->x; auto y = prev.at(0)->y; auto z = y - 1; auto next = new EventData { .x=x, .y=y, .z=z }; event->Insert(next, "third"); - success = true; + + outputs[0] = {event, 1}; + output_count = 1; status = JArrowMetrics::Status::KeepGoing; } }; -struct SumArrow : public JPipelineArrow { +struct SumArrow : public JTriggeredArrow { + + Place m_input {this, true }; + Place m_output {this, false }; double sum = 0; - SumArrow(std::string name, JMailbox* input_queue, JEventPool* pool) - : JPipelineArrow(name, false, false, true) { - this->set_input(input_queue); - this->set_output(pool); + SumArrow(std::string name, JMailbox* input_queue, JEventPool* pool) { + set_name(name); + set_is_sink(true); + attach(input_queue, 0); + attach(pool, 1); } - void process(JEvent* event, bool& success, JArrowMetrics::Status& status) { + void fire(JEvent* event, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status) { auto prev = event->Get("third"); auto z = prev.at(0)->z; sum += z; - success = true; + + outputs[0] = {event, 1}; + output_count = 1; status = JArrowMetrics::Status::KeepGoing; } }; From 3a0bef533ff159eb7459b0affdb940dc8f412b53 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 25 Oct 2024 21:40:32 -0400 Subject: [PATCH 11/22] Remove JPipelineArrow --- src/libraries/JANA/Topology/JPipelineArrow.h | 79 -------------------- 1 file changed, 79 deletions(-) delete mode 100644 src/libraries/JANA/Topology/JPipelineArrow.h diff --git a/src/libraries/JANA/Topology/JPipelineArrow.h b/src/libraries/JANA/Topology/JPipelineArrow.h deleted file mode 100644 index 225f4e995..000000000 --- a/src/libraries/JANA/Topology/JPipelineArrow.h +++ /dev/null @@ -1,79 +0,0 @@ - -// Copyright 2023, Jefferson Science Associates, LLC. -// Subject to the terms in the LICENSE file found in the top-level directory. - -#pragma once - -#include -#include -#include - - -template -class JPipelineArrow : public JArrow { -private: - Place m_input {this, true, 1, 1}; - Place m_output {this, false, 1, 1}; - -public: - JPipelineArrow(std::string name, - bool is_parallel, - bool is_source, - bool is_sink) - : JArrow(std::move(name), is_parallel, is_source, is_sink) { - } - - void set_input(JMailbox* queue) { - m_input.set_queue(queue); - } - void set_input(JEventPool* pool) { - m_input.set_pool(pool); - } - void set_output(JMailbox* queue) { - m_output.set_queue(queue); - } - void set_output(JEventPool* pool) { - m_output.set_pool(pool); - } - - void execute(JArrowMetrics& result, size_t location_id) final { - - auto start_total_time = std::chrono::steady_clock::now(); - - Data in_data {location_id}; - Data out_data {location_id}; - - bool success = m_input.pull(in_data) && m_output.pull(out_data); - if (!success) { - m_input.revert(in_data); - m_output.revert(out_data); - - auto end_total_time = std::chrono::steady_clock::now(); - result.update(JArrowMetrics::Status::ComeBackLater, 0, 1, std::chrono::milliseconds(0), end_total_time - start_total_time); - return; - } - - bool process_succeeded = true; - JArrowMetrics::Status process_status = JArrowMetrics::Status::KeepGoing; - assert(in_data.item_count == 1); - JEvent* event = in_data.items[0]; - - auto start_processing_time = std::chrono::steady_clock::now(); - static_cast(this)->process(event, process_succeeded, process_status); - auto end_processing_time = std::chrono::steady_clock::now(); - - if (process_succeeded) { - in_data.item_count = 0; - out_data.item_count = 1; - out_data.items[0] = event; - } - m_input.push(in_data); - m_output.push(out_data); - - // Publish metrics - auto end_total_time = std::chrono::steady_clock::now(); - auto latency = (end_processing_time - start_processing_time); - auto overhead = (end_total_time - start_total_time) - latency; - result.update(process_status, process_succeeded, 1, latency, overhead); - } -}; From d2cb9042246b180fa386780bed7d38ff1ac73f3b Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 25 Oct 2024 22:30:39 -0400 Subject: [PATCH 12/22] JTriggeredArrow supports no-input-event and timed triggers --- src/libraries/JANA/Topology/JArrow.h | 1 - src/libraries/JANA/Topology/JTriggeredArrow.h | 26 +++++++++++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/libraries/JANA/Topology/JArrow.h b/src/libraries/JANA/Topology/JArrow.h index 579d12ccf..7754a3dde 100644 --- a/src/libraries/JANA/Topology/JArrow.h +++ b/src/libraries/JANA/Topology/JArrow.h @@ -40,7 +40,6 @@ class JArrow { protected: JLogger m_logger; std::vector m_places; // Will eventually supplant m_listeners - int m_next_input_port=0; public: std::string get_name() { return m_name; } diff --git a/src/libraries/JANA/Topology/JTriggeredArrow.h b/src/libraries/JANA/Topology/JTriggeredArrow.h index 10759096f..eb847dab7 100644 --- a/src/libraries/JANA/Topology/JTriggeredArrow.h +++ b/src/libraries/JANA/Topology/JTriggeredArrow.h @@ -2,18 +2,33 @@ #pragma once #include +#include template struct JTriggeredArrow : public JArrow { + using clock_t = std::chrono::steady_clock; + + int m_next_input_port=0; // -1 denotes "no input necessary", e.g. for barrier events + clock_t::time_point m_next_visit_time=clock_t::now(); + void execute(JArrowMetrics& result, size_t location_id) final { auto start_total_time = std::chrono::steady_clock::now(); - JEvent* input = pull(m_next_input_port, location_id); + if (m_next_visit_time > start_total_time) { + // If we haven't reached the next visit time, exit immediately + result.update(JArrowMetrics::Status::ComeBackLater, 0, 0, std::chrono::milliseconds(0), std::chrono::milliseconds(0)); + return; + } + + JEvent* input = nullptr; + if (m_next_input_port != -1) { + input = pull(m_next_input_port, location_id); + } - if (input == nullptr) { + if (input == nullptr && m_next_input_port != -1) { // Failed to obtain the input we needed; arrow is NOT ready to fire auto end_total_time = std::chrono::steady_clock::now(); @@ -22,14 +37,15 @@ struct JTriggeredArrow : public JArrow { } else { // Obtained the input we needed; arrow is ready to fire + // Remember that `input` might be nullptr, in case arrow doesn't need any input event auto start_processing_time = std::chrono::steady_clock::now(); OutputData outputs; size_t output_count; - JArrowMetrics::Status process_status = JArrowMetrics::Status::KeepGoing; + JArrowMetrics::Status status = JArrowMetrics::Status::KeepGoing; - static_cast(this)->fire(input, outputs, output_count, process_status); + static_cast(this)->fire(input, outputs, output_count, status); auto end_processing_time = std::chrono::steady_clock::now(); @@ -39,7 +55,7 @@ struct JTriggeredArrow : public JArrow { auto latency = (end_processing_time - start_processing_time); auto overhead = (end_total_time - start_total_time) - latency; - result.update(process_status, 1, 1, latency, overhead); + result.update(status, 1, 1, latency, overhead); } } }; From abb5ba562a21c33717bd5696541a9ad183382190 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Fri, 25 Oct 2024 22:46:03 -0400 Subject: [PATCH 13/22] JTriggeredArrow handles 'rejected' events correctly --- src/libraries/JANA/Topology/JTriggeredArrow.h | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/src/libraries/JANA/Topology/JTriggeredArrow.h b/src/libraries/JANA/Topology/JTriggeredArrow.h index eb847dab7..c9ea10b6c 100644 --- a/src/libraries/JANA/Topology/JTriggeredArrow.h +++ b/src/libraries/JANA/Topology/JTriggeredArrow.h @@ -34,29 +34,38 @@ struct JTriggeredArrow : public JArrow { auto end_total_time = std::chrono::steady_clock::now(); result.update(JArrowMetrics::Status::ComeBackLater, 0, 1, std::chrono::milliseconds(0), end_total_time - start_total_time); + return; } - else { - // Obtained the input we needed; arrow is ready to fire - // Remember that `input` might be nullptr, in case arrow doesn't need any input event - - auto start_processing_time = std::chrono::steady_clock::now(); - OutputData outputs; - size_t output_count; - JArrowMetrics::Status status = JArrowMetrics::Status::KeepGoing; + // Obtained the input we needed; arrow is ready to fire + // Remember that `input` might be nullptr, in case arrow doesn't need any input event - static_cast(this)->fire(input, outputs, output_count, status); + auto start_processing_time = std::chrono::steady_clock::now(); - auto end_processing_time = std::chrono::steady_clock::now(); + OutputData outputs; + size_t output_count; + JArrowMetrics::Status status = JArrowMetrics::Status::KeepGoing; - push(outputs, output_count, location_id); + static_cast(this)->fire(input, outputs, output_count, status); - auto end_total_time = std::chrono::steady_clock::now(); + auto end_processing_time = std::chrono::steady_clock::now(); - auto latency = (end_processing_time - start_processing_time); - auto overhead = (end_total_time - start_total_time) - latency; - result.update(status, 1, 1, latency, overhead); + // Even though fire() returns an output count, some of those 'outputs' might have been sent right back to + // their input pool. These mustn't be counted as "processed" in the metrics. + size_t processed_count = 0; + for (size_t output=0; outputis_input) { + processed_count++; + } } + + push(outputs, output_count, location_id); + + auto end_total_time = std::chrono::steady_clock::now(); + + auto latency = (end_processing_time - start_processing_time); + auto overhead = (end_total_time - start_total_time) - latency; + result.update(status, processed_count, 1, latency, overhead); } }; From 65e52468f6c6012989a0648a97c29fe0fbe05fb7 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sat, 26 Oct 2024 01:06:26 -0400 Subject: [PATCH 14/22] Migrate JEventSourceArrow to JTriggeredArrow --- .../JANA/Topology/JEventSourceArrow.cc | 128 +++++++----------- .../JANA/Topology/JEventSourceArrow.h | 11 +- 2 files changed, 53 insertions(+), 86 deletions(-) diff --git a/src/libraries/JANA/Topology/JEventSourceArrow.cc b/src/libraries/JANA/Topology/JEventSourceArrow.cc index 7fa90eff8..de034a9a3 100644 --- a/src/libraries/JANA/Topology/JEventSourceArrow.cc +++ b/src/libraries/JANA/Topology/JEventSourceArrow.cc @@ -3,6 +3,7 @@ // Subject to the terms in the LICENSE file found in the top-level directory. +#include "JANA/Topology/JArrowMetrics.h" #include #include #include @@ -10,67 +11,13 @@ JEventSourceArrow::JEventSourceArrow(std::string name, std::vector sources) - : JArrow(name, false, true, false), m_sources(sources) { + : m_sources(sources) { + set_name(name); + set_is_source(true); } -void JEventSourceArrow::execute(JArrowMetrics& result, size_t location_id) { - - auto start_total_time = std::chrono::steady_clock::now(); - - Data in_data {location_id}; - Data out_data {location_id}; - - bool success = m_input.pull(in_data) && m_output.pull(out_data); - if (!success) { - m_input.revert(in_data); - m_output.revert(out_data); - // TODO: Test that revert works properly - - auto end_total_time = std::chrono::steady_clock::now(); - result.update(JArrowMetrics::Status::ComeBackLater, 0, 1, std::chrono::milliseconds(0), end_total_time - start_total_time); - return; - } - - bool process_succeeded = true; - JArrowMetrics::Status process_status = JArrowMetrics::Status::KeepGoing; - - // If we have a pending barrier event, the input event will just be nullptr - JEvent* event_in = (in_data.item_count == 1) ? in_data.items[0] : nullptr; - - auto start_processing_time = std::chrono::steady_clock::now(); - JEvent* event_out = process(event_in, process_succeeded, process_status); - auto end_processing_time = std::chrono::steady_clock::now(); - - if (process_succeeded) { - if (event_out == nullptr) { - // Event will be nullptr if the JEventSource emitted a barrier event that must - // be held in m_pending_barrier_event until all preceding events have finished - in_data.item_count = 0; // Nothing gets returned to the input queue - out_data.item_count = 0; // Nothing gets sent to the output queue - m_input.min_item_count = 0; // We don't ask for any events from the input queue next time - m_input.max_item_count = 0; - } - else { - in_data.item_count = 0; // Nothing gets returned to the input queue - out_data.item_count = 1; // Event gets sent to the output queue - out_data.items[0] = event_out; - m_input.min_item_count = 1; // We ask for a fresh event from the input queue next time - m_input.max_item_count = 1; - } - } - m_input.push(in_data); - m_output.push(out_data); - - // Publish metrics - auto end_total_time = std::chrono::steady_clock::now(); - auto latency = (end_processing_time - start_processing_time); - auto overhead = (end_total_time - start_total_time) - latency; - result.update(process_status, process_succeeded, 1, latency, overhead); -} - - -JEvent* JEventSourceArrow::process(JEvent* event, bool& success, JArrowMetrics::Status& arrow_status) { +void JEventSourceArrow::fire(JEvent* event, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status) { LOG_DEBUG(m_logger) << "Executing arrow " << get_name() << LOG_END; @@ -85,17 +32,25 @@ JEvent* JEventSourceArrow::process(JEvent* event, bool& success, JArrowMetrics:: // Topology has drained; only remaining in-flight event is the barrier event itself, // which we have held on to until now - JEvent* barrier_event = m_pending_barrier_event; + outputs[0] = {m_pending_barrier_event, 1}; + output_count = 1; + status = JArrowMetrics::Status::KeepGoing; + m_pending_barrier_event = nullptr; - return barrier_event; + // There's not much for the thread team to do while the barrier event makes its way through the topology. + // Eventually we might be able to use this to communicate to the scheduler to not wake threads whose only + // available action is to hammer the JEventSourceArrow + return; } else { // Topology has _not_ finished draining, all we can do is wait LOG_DEBUG(m_logger) << "JEventSourceArrow: Waiting on pending barrier event" << LOG_END; LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " with result ComeBackLater"<< LOG_END; - arrow_status = JArrowMetrics::Status::ComeBackLater; - success = false; - return nullptr; + + assert(event == nullptr); + output_count = 0; + status = JArrowMetrics::Status::ComeBackLater; + return; } } else { @@ -103,20 +58,24 @@ JEvent* JEventSourceArrow::process(JEvent* event, bool& success, JArrowMetrics:: // until it is finished before emitting any more events if (m_sources[m_current_source]->GetFinishedEventCount() == m_sources[m_current_source]->GetEmittedEventCount()) { - - LOG_DEBUG(m_logger) << "JEventSourceArrow: Barrier event finished, returning to normal operation" << LOG_END; - // Barrier event has finished + // Barrier event has finished. + LOG_DEBUG(m_logger) << "JEventSourceArrow: Barrier event finished, returning to normal operation" << LOG_END; m_barrier_active = false; - // Continue to emit the next event + m_next_input_port = 0; + + output_count = 0; + status = JArrowMetrics::Status::KeepGoing; + return; } else { // Barrier event has NOT finished LOG_DEBUG(m_logger) << "JEventSourceArrow: Waiting on in-flight barrier event" << LOG_END; LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " with result ComeBackLater"<< LOG_END; - success = false; - arrow_status = JArrowMetrics::Status::ComeBackLater; - return nullptr; + assert(event == nullptr); + output_count = 0; + status = JArrowMetrics::Status::ComeBackLater; + return; } } } @@ -133,28 +92,37 @@ JEvent* JEventSourceArrow::process(JEvent* event, bool& success, JArrowMetrics:: else if (source_status == JEventSource::Result::FailureTryAgain){ // This JEventSource isn't finished yet, so we obtained either Success or TryAgainLater LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " with result ComeBackLater"<< LOG_END; - success = false; - arrow_status = JArrowMetrics::Status::ComeBackLater; - return event; + outputs[0] = {event, 0}; // Reject + output_count = 1; + status = JArrowMetrics::Status::ComeBackLater; + return; } else if (event->GetSequential()){ // Source succeeded, but returned a barrier event LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " with result Success, holding back barrier event# " << event->GetEventNumber() << LOG_END; m_pending_barrier_event = event; m_barrier_active = true; - return nullptr; + m_next_input_port = -1; // Stop popping events from the input queue until barrier event has finished + + // Arrow hangs on to the barrier event until the topology fully drains + output_count = 0; + status = JArrowMetrics::Status::KeepGoing; // Mysteriously livelocks if we set this to ComeBackLater?? + return; } else { // Source succeeded, did NOT return a barrier event LOG_DEBUG(m_logger) << "Executed arrow " << get_name() << " with result Success, emitting event# " << event->GetEventNumber() << LOG_END; - success = true; - arrow_status = JArrowMetrics::Status::KeepGoing; - return event; + outputs[0] = {event, 1}; // SUCCESS! + output_count = 1; + status = JArrowMetrics::Status::KeepGoing; + return; } } - success = false; - arrow_status = JArrowMetrics::Status::Finished; - return event; + + // All event sources have finished now + outputs[0] = {event, 0}; // Reject event + output_count = 1; + status = JArrowMetrics::Status::Finished; } void JEventSourceArrow::initialize() { diff --git a/src/libraries/JANA/Topology/JEventSourceArrow.h b/src/libraries/JANA/Topology/JEventSourceArrow.h index 655881d45..78acbda58 100644 --- a/src/libraries/JANA/Topology/JEventSourceArrow.h +++ b/src/libraries/JANA/Topology/JEventSourceArrow.h @@ -3,18 +3,18 @@ // Subject to the terms in the LICENSE file found in the top-level directory. #pragma once -#include +#include -class JEventSourceArrow : public JArrow { +class JEventSourceArrow : public JTriggeredArrow { private: std::vector m_sources; size_t m_current_source = 0; bool m_barrier_active = false; JEvent* m_pending_barrier_event = nullptr; - Place m_input {this, true, 1, 1}; - Place m_output {this, false, 1, 1}; + Place m_input {this, true}; + Place m_output {this, false}; public: JEventSourceArrow(std::string name, std::vector sources); @@ -34,7 +34,6 @@ class JEventSourceArrow : public JArrow { void initialize() final; void finalize() final; - void execute(JArrowMetrics& result, size_t location_id) final; - JEvent* process(JEvent* event, bool& success, JArrowMetrics::Status& status); + void fire(JEvent* input, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status); }; From 8454a43e7dacf5a906bd0e2957a6db24a76a06b6 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sat, 26 Oct 2024 23:41:19 -0400 Subject: [PATCH 15/22] Migrate JUnfoldArrow to JTriggeredArrow --- src/libraries/JANA/Topology/JUnfoldArrow.h | 203 ++++++++---------- .../unit_tests/Components/UnfoldTests.cc | 3 +- 2 files changed, 91 insertions(+), 115 deletions(-) diff --git a/src/libraries/JANA/Topology/JUnfoldArrow.h b/src/libraries/JANA/Topology/JUnfoldArrow.h index 31929dc1b..aab5c4521 100644 --- a/src/libraries/JANA/Topology/JUnfoldArrow.h +++ b/src/libraries/JANA/Topology/JUnfoldArrow.h @@ -3,35 +3,31 @@ #pragma once -#include +#include "JANA/Topology/JArrowMetrics.h" +#include #include -class JUnfoldArrow : public JArrow { -private: +class JUnfoldArrow : public JTriggeredArrow { +public: + const int PARENT_IN = 0; + const int CHILD_IN = 1; + const int CHILD_OUT = 2; +private: JEventUnfolder* m_unfolder = nullptr; JEvent* m_parent_event = nullptr; - bool m_ready_to_fetch_parent = true; + JEvent* m_child_event = nullptr; - Place m_parent_in; - Place m_child_in; - Place m_child_out; + Place m_parent_in {this, true}; + Place m_child_in {this, true}; + Place m_child_out {this, false}; public: - - JUnfoldArrow( - std::string name, - JEventUnfolder* unfolder) - - : JArrow(std::move(name), false, false, false), - m_unfolder(unfolder), - m_parent_in(this, true, 1, 1), - m_child_in(this, true, 1, 1), - m_child_out(this, false, 1, 1) - { + JUnfoldArrow(std::string name, JEventUnfolder* unfolder) : m_unfolder(unfolder) { + set_name(name); + m_next_input_port = PARENT_IN; } - void attach_parent_in(JMailbox* parent_in) { m_parent_in.place_ref = parent_in; m_parent_in.is_queue = true; @@ -63,35 +59,6 @@ class JUnfoldArrow : public JArrow { LOG_INFO(m_logger) << "Finalized JEventUnfolder '" << m_unfolder->GetTypeName() << "'" << LOG_END; } - bool try_pull_all(Data& pi, Data& ci, Data& co) { - bool success; - success = m_parent_in.pull(pi); - if (! success) { - return false; - } - success = m_child_in.pull(ci); - if (! success) { - m_parent_in.push(pi); - return false; - } - success = m_child_out.pull(co); - if (! success) { - m_parent_in.push(pi); - m_child_in.push(ci); - return false; - } - return true; - } - - size_t push_all(Data& parent_in, Data& child_in, Data& child_out) { - size_t message_count = 0; - message_count += m_parent_in.push(parent_in); - message_count += m_child_in.push(child_in); - message_count += m_child_out.push(child_out); - return message_count; - } - - size_t get_pending() final { size_t sum = 0; for (Place* place : m_places) { @@ -104,78 +71,86 @@ class JUnfoldArrow : public JArrow { return sum; } + void fire(JEvent* event, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status) { - void execute(JArrowMetrics& metrics, size_t location_id) final { - - auto start_total_time = std::chrono::steady_clock::now(); - - Data parent_in_data {location_id}; - Data child_in_data {location_id}; - Data child_out_data {location_id}; - - bool success = try_pull_all(parent_in_data, child_in_data, child_out_data); - if (success) { - - auto start_processing_time = std::chrono::steady_clock::now(); - if (m_ready_to_fetch_parent) { - m_ready_to_fetch_parent = false; - m_parent_in.min_item_count = 0; - m_parent_in.max_item_count = 0; - m_parent_event = parent_in_data.items[0]; - parent_in_data.items[0] = nullptr; - parent_in_data.item_count = 0; - } - auto child = child_in_data.items[0]; - child_in_data.items[0] = nullptr; - child_in_data.item_count = 0; - if (m_parent_event == nullptr) { - throw JException("Attempting to unfold without a valid parent event"); - } - if (m_parent_event->GetLevel() != m_unfolder->GetLevel()) { - throw JException("JUnfolder: Expected parent with level %d, got %d", m_unfolder->GetLevel(), m_parent_event->GetLevel()); - } - if (child->GetLevel() != m_unfolder->GetChildLevel()) { - throw JException("JUnfolder: Expected child with level %d, got %d", m_unfolder->GetChildLevel(), child->GetLevel()); - } - - auto status = m_unfolder->DoUnfold(*m_parent_event, *child); - - - // Join always succeeds (for now) - child->SetParent(m_parent_event); - - LOG_DEBUG(m_logger) << "Unfold succeeded: Parent event = " << m_parent_event->GetEventNumber() << ", child event = " << child->GetEventNumber() << LOG_END; - // TODO: We'll need something more complicated for the streaming join case - - if (status == JEventUnfolder::Result::NextChildNextParent || status == JEventUnfolder::Result::KeepChildNextParent) { - LOG_DEBUG(m_logger) << "Unfold finished with parent event = " << m_parent_event->GetEventNumber() << LOG_END; - m_ready_to_fetch_parent = true; - m_parent_event->Release(); - m_parent_event = nullptr; - m_parent_in.min_item_count = 1; - m_parent_in.max_item_count = 1; - } - - child_out_data.items[0] = child; - child_out_data.item_count = 1; - - auto end_processing_time = std::chrono::steady_clock::now(); - size_t events_processed = push_all(parent_in_data, child_in_data, child_out_data); - - auto end_total_time = std::chrono::steady_clock::now(); - auto latency = (end_processing_time - start_processing_time); - auto overhead = (end_total_time - start_total_time) - latency; - - metrics.update(JArrowMetrics::Status::KeepGoing, events_processed, 1, latency, overhead); - return; + // Take whatever we were given + if (this->m_next_input_port == PARENT_IN) { + assert(m_parent_event == nullptr); + m_parent_event = event; + } + else if (this->m_next_input_port == CHILD_IN) { + assert(m_child_event == nullptr); + m_child_event = event; } else { - auto end_total_time = std::chrono::steady_clock::now(); - metrics.update(JArrowMetrics::Status::ComeBackLater, 0, 1, std::chrono::milliseconds(0), end_total_time - start_total_time); + throw JException("Invalid input port for JEventUnfolder!"); + } + + // Check if we should exit early because we don't have a parent event + if (m_parent_event == nullptr) { + m_next_input_port = PARENT_IN; + output_count = 0; + status = JArrowMetrics::Status::KeepGoing; return; } - } + // Check if we should exit early because we don't have a child event + if (m_child_event == nullptr) { + m_next_input_port = CHILD_IN; + output_count = 0; + status = JArrowMetrics::Status::KeepGoing; + return; + } + + // At this point we know we have both inputs, so we can run the unfolder. First we validate + // that the events we received are at the correct level for the unfolder. Hopefully the only + // way to end up here is to override the JTopologyBuilder wiring and do it wrong + + if (m_parent_event->GetLevel() != m_unfolder->GetLevel()) { + throw JException("JUnfolder: Expected parent with level %d, got %d", m_unfolder->GetLevel(), m_parent_event->GetLevel()); + } + + if (m_child_event->GetLevel() != m_unfolder->GetChildLevel()) { + throw JException("JUnfolder: Expected child with level %d, got %d", m_unfolder->GetChildLevel(), m_child_event->GetLevel()); + } + + auto result = m_unfolder->DoUnfold(*m_parent_event, *m_child_event); + LOG_DEBUG(m_logger) << "Unfold succeeded: Parent event = " << m_parent_event->GetEventNumber() << ", child event = " << m_child_event->GetEventNumber() << LOG_END; + + if (result == JEventUnfolder::Result::KeepChildNextParent) { + m_parent_event->Release(); // Decrement the reference count so that this can be recycled + m_parent_event = nullptr; + output_count = 0; + m_next_input_port = PARENT_IN; + status = JArrowMetrics::Status::KeepGoing; + LOG_DEBUG(m_logger) << "Unfold finished with parent event = " << m_parent_event->GetEventNumber() << LOG_END; + return; + } + else if (result == JEventUnfolder::Result::NextChildKeepParent) { + m_child_event->SetParent(m_parent_event); + outputs[0] = {m_child_event, CHILD_OUT}; + output_count = 1; + m_child_event = nullptr; + m_next_input_port = CHILD_IN; + status = JArrowMetrics::Status::KeepGoing; + return; + } + else if (result == JEventUnfolder::Result::NextChildNextParent) { + m_child_event->SetParent(m_parent_event); + m_parent_event->Release(); // Decrement the reference count so that this can be recycled + outputs[0] = {m_child_event, CHILD_OUT}; + output_count = 1; + m_child_event = nullptr; + m_parent_event = nullptr; + m_next_input_port = PARENT_IN; + status = JArrowMetrics::Status::KeepGoing; + LOG_DEBUG(m_logger) << "Unfold finished with parent event = " << m_parent_event->GetEventNumber() << LOG_END; + return; + } + else { + throw JException("Unsupported (corrupt?) JEventUnfolder::Result"); + } + } }; diff --git a/src/programs/unit_tests/Components/UnfoldTests.cc b/src/programs/unit_tests/Components/UnfoldTests.cc index 371d095cd..cdb048af0 100644 --- a/src/programs/unit_tests/Components/UnfoldTests.cc +++ b/src/programs/unit_tests/Components/UnfoldTests.cc @@ -66,7 +66,8 @@ TEST_CASE("UnfoldTests_Basic") { JArrowMetrics m; arrow.initialize(); - arrow.execute(m, 0); + arrow.execute(m, 0); // First call to execute() picks up the parent and exits early + arrow.execute(m, 0); // Second call to execute() picks up the child, calls Unfold(), and emits the newly parented child REQUIRE(m.get_last_status() == JArrowMetrics::Status::KeepGoing); REQUIRE(child_queue.size() == 1); REQUIRE(unfolder.preprocessed_event_nrs.size() == 0); From 48d97bfe568971a3362f181d1239ae8c9eeb8686 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sun, 27 Oct 2024 01:04:25 -0400 Subject: [PATCH 16/22] Migrate JFoldArrow to JTriggeredArrow --- src/libraries/JANA/Topology/JFoldArrow.h | 122 ++++++++--------------- 1 file changed, 39 insertions(+), 83 deletions(-) diff --git a/src/libraries/JANA/Topology/JFoldArrow.h b/src/libraries/JANA/Topology/JFoldArrow.h index 47f17a95f..75e5bc8b8 100644 --- a/src/libraries/JANA/Topology/JFoldArrow.h +++ b/src/libraries/JANA/Topology/JFoldArrow.h @@ -3,20 +3,25 @@ #pragma once -#include -#include +#include "JANA/Topology/JArrowMetrics.h" +#include + +class JFoldArrow : public JTriggeredArrow { +public: + const int CHILD_IN = 0; + const int CHILD_OUT = 1; + const int PARENT_OUT = 2; -class JFoldArrow : public JArrow { private: // TODO: Support user-provided folders // JEventFolder* m_folder = nullptr; - + JEventLevel m_parent_level; JEventLevel m_child_level; - Place m_child_in; - Place m_child_out; - Place m_parent_out; + Place m_child_in {this, true}; + Place m_child_out {this, false}; + Place m_parent_out {this, false}; public: JFoldArrow( @@ -24,14 +29,15 @@ class JFoldArrow : public JArrow { JEventLevel parent_level, JEventLevel child_level) - : JArrow(std::move(name), false, false, false), - // m_folder(folder), + : // m_folder(folder), m_parent_level(parent_level), m_child_level(child_level), m_child_in(this, true, 1, 1), m_child_out(this, false, 1, 1), m_parent_out(this, false, 1, 1) { + set_name(name); + m_next_input_port = CHILD_IN; } void attach_child_in(JMailbox* child_in) { @@ -54,13 +60,11 @@ class JFoldArrow : public JArrow { m_parent_out.is_queue = false; } - void attach_parent_out(JMailbox* parent_out) { m_parent_out.place_ref = parent_out; m_parent_out.is_queue = true; } - void initialize() final { /* if (m_folder != nullptr) { @@ -83,81 +87,33 @@ class JFoldArrow : public JArrow { LOG_INFO(m_logger) << "Finalized JEventFolder (trivial)" << LOG_END; } - bool try_pull_all(Data& ci, Data& co, Data& po) { - bool success; - success = m_child_in.pull(ci); - if (! success) { - return false; - } - success = m_child_out.pull(co); - if (! success) { - return false; - } - success = m_parent_out.pull(po); - if (! success) { - return false; - } - return true; - } + void fire(JEvent* event, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status) { - size_t push_all(Data& ci, Data& co, Data& po) { - size_t message_count = co.item_count; - m_child_in.push(ci); - m_child_out.push(co); - m_parent_out.push(po); - return message_count; - } + assert(m_next_input_port == CHILD_IN); - void execute(JArrowMetrics& metrics, size_t location_id) final { - - auto start_total_time = std::chrono::steady_clock::now(); - - Data child_in_data {location_id}; - Data child_out_data {location_id}; - Data parent_out_data {location_id}; - - bool success = try_pull_all(child_in_data, child_out_data, parent_out_data); - if (success) { - - auto start_processing_time = std::chrono::steady_clock::now(); - auto child = child_in_data.items[0]; - child_in_data.items[0] = nullptr; - child_in_data.item_count = 0; - if (child->GetLevel() != m_child_level) { - throw JException("JFoldArrow received a child with the wrong event level"); - } - - // TODO: Call folders here - auto* parent = child->ReleaseParent(m_parent_level); - - // Put child on the output queue - child_out_data.items[0] = child; - child_out_data.item_count = 1; - - // Only recycle the parent once the reference count hits zero - if (parent != nullptr) { - parent_out_data.items[0] = parent; - parent_out_data.item_count = 1; - } - else { - parent_out_data.items[0] = nullptr; - parent_out_data.item_count = 0; - } - - auto end_processing_time = std::chrono::steady_clock::now(); - size_t events_processed = push_all(child_in_data, child_out_data, parent_out_data); - - auto end_total_time = std::chrono::steady_clock::now(); - auto latency = (end_processing_time - start_processing_time); - auto overhead = (end_total_time - start_total_time) - latency; - - metrics.update(JArrowMetrics::Status::KeepGoing, events_processed, 1, latency, overhead); - return; + // Check that child is at the correct event level + if (event->GetLevel() != m_child_level) { + throw JException("JFoldArrow received a child with the wrong event level"); } - else { - auto end_total_time = std::chrono::steady_clock::now(); - metrics.update(JArrowMetrics::Status::ComeBackLater, 0, 1, std::chrono::milliseconds(0), end_total_time - start_total_time); - return; + + // TODO: Call folders here + // auto parent = child->GetParent(m_parent_level); + // m_folder->Fold(*child, *parent); + + status = JArrowMetrics::Status::KeepGoing; + outputs[0] = {event, CHILD_OUT}; + output_count = 1; + + auto* parent = event->ReleaseParent(m_parent_level); + if (parent != nullptr) { + // JEvent::ReleaseParent() returns nullptr if there are remaining references + // to the parent event. If non-null, we are completely done with the parent + // and are free to return it to the pool. In the future we could have the pool + // itself handle the logic for releasing parents, in which case we could avoid + // trivial JEventFolders. + + outputs[1] = {parent, PARENT_OUT}; + output_count = 2; } } From 444828285a2fbfdf37b50fd16a28c02dae6bc76a Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sun, 27 Oct 2024 03:06:11 -0400 Subject: [PATCH 17/22] Rough cut of JEventFolder --- src/libraries/JANA/JEventFolder.h | 164 +++++++++++++++++++++++ src/libraries/JANA/Topology/JFoldArrow.h | 37 +++-- 2 files changed, 182 insertions(+), 19 deletions(-) create mode 100644 src/libraries/JANA/JEventFolder.h diff --git a/src/libraries/JANA/JEventFolder.h b/src/libraries/JANA/JEventFolder.h new file mode 100644 index 000000000..d33dbc692 --- /dev/null +++ b/src/libraries/JANA/JEventFolder.h @@ -0,0 +1,164 @@ +// Copyright 2024, Jefferson Science Associates, LLC. +// Subject to the terms in the LICENSE file found in the top-level directory. + +#pragma once + +#include +#include +#include +#include +#include + +class JApplication; +class JEventFolder : public jana::components::JComponent, + public jana::components::JHasRunCallbacks, + public jana::components::JHasInputs, + public jana::components::JHasOutputs { + +private: + int32_t m_last_run_number = -1; + JEventLevel m_child_level; + bool m_call_preprocess_upstream = true; + + +public: + + JEventFolder() = default; + virtual ~JEventFolder() {}; + + virtual void Init() {}; + + virtual void Preprocess(const JEvent& /*parent*/) const {}; + + virtual void Fold(const JEvent& /*child*/, JEvent& /*parent*/, int /*item_nr*/) { + throw JException("Not implemented yet!"); + }; + + virtual void Finish() {}; + + + // Configuration + + void SetParentLevel(JEventLevel level) { m_level = level; } + + void SetChildLevel(JEventLevel level) { m_child_level = level; } + + void SetCallPreprocessUpstream(bool call_upstream) { m_call_preprocess_upstream = call_upstream; } + + JEventLevel GetChildLevel() { return m_child_level; } + + + public: + // Backend + + void DoInit() { + std::lock_guard lock(m_mutex); + if (m_status != Status::Uninitialized) { + throw JException("JEventFolder: Attempting to initialize twice or from an invalid state"); + } + // TODO: Obtain overrides of collection names from param manager + for (auto* parameter : m_parameters) { + parameter->Configure(*(m_app->GetJParameterManager()), m_prefix); + } + for (auto* service : m_services) { + service->Fetch(m_app); + } + CallWithJExceptionWrapper("JEventFolder::Init", [&](){Init();}); + m_status = Status::Initialized; + } + + void DoPreprocess(const JEvent& child) { + { + std::lock_guard lock(m_mutex); + if (m_status != Status::Initialized) { + throw JException("JEventFolder: Component needs to be initialized and not finalized before Fold can be called"); + // TODO: Consider calling Initialize(with_lock=false) like we do elsewhere + } + } + for (auto* input : m_inputs) { + input->PrefetchCollection(child); + } + if (m_callback_style != CallbackStyle::DeclarativeMode) { + CallWithJExceptionWrapper("JEventFolder::Preprocess", [&](){ + Preprocess(child); + }); + } + } + + void DoFold(const JEvent& child, JEvent& parent) { + std::lock_guard lock(m_mutex); + if (m_status != Status::Initialized) { + throw JException("Component needs to be initialized and not finalized before Fold() can be called"); + } + if (!m_call_preprocess_upstream) { + CallWithJExceptionWrapper("JEventFolder::Preprocess", [&](){ + Preprocess(parent); + }); + } + if (m_last_run_number != parent.GetRunNumber()) { + for (auto* resource : m_resources) { + resource->ChangeRun(parent.GetRunNumber(), m_app); + } + if (m_callback_style == CallbackStyle::DeclarativeMode) { + CallWithJExceptionWrapper("JEventFolder::ChangeRun", [&](){ + ChangeRun(parent.GetRunNumber()); + }); + } + else { + CallWithJExceptionWrapper("JEventFolder::ChangeRun", [&](){ + ChangeRun(parent); + }); + } + m_last_run_number = parent.GetRunNumber(); + } + for (auto* input : m_inputs) { + input->GetCollection(parent); + // TODO: This requires that all inputs come from the parent. + // However, eventually we will want to support inputs + // that come from the child. + } + for (auto* output : m_outputs) { + output->Reset(); + } + auto child_number = child.GetEventIndex(); + CallWithJExceptionWrapper("JEventFolder::Fold", [&](){ + Fold(child, parent, child_number); + }); + + for (auto* output : m_outputs) { + output->InsertCollection(parent); + } + } + + void DoFinish() { + std::lock_guard lock(m_mutex); + if (m_status != Status::Finalized) { + CallWithJExceptionWrapper("JEventFolder::Finish", [&](){ + Finish(); + }); + m_status = Status::Finalized; + } + } + + void Summarize(JComponentSummary& summary) const override { + auto* us = new JComponentSummary::Component( + "Folder", GetPrefix(), GetTypeName(), GetLevel(), GetPluginName()); + + for (const auto* input : m_inputs) { + size_t subinput_count = input->names.size(); + for (size_t i=0; iAddInput(new JComponentSummary::Collection("", input->names[i], input->type_name, input->levels[i])); + } + } + for (const auto* output : m_outputs) { + size_t suboutput_count = output->collection_names.size(); + for (size_t i=0; iAddOutput(new JComponentSummary::Collection("", output->collection_names[i], output->type_name, GetLevel())); + } + } + summary.Add(us); + } + +}; + + diff --git a/src/libraries/JANA/Topology/JFoldArrow.h b/src/libraries/JANA/Topology/JFoldArrow.h index 75e5bc8b8..4bd12248b 100644 --- a/src/libraries/JANA/Topology/JFoldArrow.h +++ b/src/libraries/JANA/Topology/JFoldArrow.h @@ -3,8 +3,8 @@ #pragma once -#include "JANA/Topology/JArrowMetrics.h" #include +#include class JFoldArrow : public JTriggeredArrow { public: @@ -13,8 +13,7 @@ class JFoldArrow : public JTriggeredArrow { const int PARENT_OUT = 2; private: - // TODO: Support user-provided folders - // JEventFolder* m_folder = nullptr; + JEventFolder* m_folder = nullptr; JEventLevel m_parent_level; JEventLevel m_child_level; @@ -29,8 +28,7 @@ class JFoldArrow : public JTriggeredArrow { JEventLevel parent_level, JEventLevel child_level) - : // m_folder(folder), - m_parent_level(parent_level), + : m_parent_level(parent_level), m_child_level(child_level), m_child_in(this, true, 1, 1), m_child_out(this, false, 1, 1), @@ -40,6 +38,10 @@ class JFoldArrow : public JTriggeredArrow { m_next_input_port = CHILD_IN; } + void set_folder(JEventFolder* folder) { + m_folder = folder; + } + void attach_child_in(JMailbox* child_in) { m_child_in.place_ref = child_in; m_child_in.is_queue = true; @@ -66,25 +68,23 @@ class JFoldArrow : public JTriggeredArrow { } void initialize() final { - /* if (m_folder != nullptr) { m_folder->DoInit(); - LOG_INFO(m_logger) << "Initialized JEventFolder '" << m_unfolder->GetTypeName() << "'" << LOG_END; + LOG_INFO(m_logger) << "Initialized JEventFolder '" << m_folder->GetTypeName() << "'" << LOG_END; } else { + LOG_INFO(m_logger) << "Initialized JEventFolder (trivial)" << LOG_END; } - */ - LOG_INFO(m_logger) << "Initialized JEventFolder (trivial)" << LOG_END; } void finalize() final { - /* if (m_folder != nullptr) { m_folder->DoFinish(); - LOG_INFO(m_logger) << "Finalized JEventFolder '" << m_unfolder->GetTypeName() << "'" << LOG_END; + LOG_INFO(m_logger) << "Finalized JEventFolder '" << m_folder->GetTypeName() << "'" << LOG_END; + } + else { + LOG_INFO(m_logger) << "Finalized JEventFolder (trivial)" << LOG_END; } - */ - LOG_INFO(m_logger) << "Finalized JEventFolder (trivial)" << LOG_END; } void fire(JEvent* event, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status) { @@ -96,23 +96,22 @@ class JFoldArrow : public JTriggeredArrow { throw JException("JFoldArrow received a child with the wrong event level"); } - // TODO: Call folders here - // auto parent = child->GetParent(m_parent_level); - // m_folder->Fold(*child, *parent); + auto parent = const_cast(&event->GetParent(m_parent_level)); + m_folder->DoFold(*event, *parent); status = JArrowMetrics::Status::KeepGoing; outputs[0] = {event, CHILD_OUT}; output_count = 1; - auto* parent = event->ReleaseParent(m_parent_level); - if (parent != nullptr) { + auto* released_parent = event->ReleaseParent(m_parent_level); + if (released_parent != nullptr) { // JEvent::ReleaseParent() returns nullptr if there are remaining references // to the parent event. If non-null, we are completely done with the parent // and are free to return it to the pool. In the future we could have the pool // itself handle the logic for releasing parents, in which case we could avoid // trivial JEventFolders. - outputs[1] = {parent, PARENT_OUT}; + outputs[1] = {released_parent, PARENT_OUT}; output_count = 2; } } From ff77aa0483e1f274bdb205e1b00d99894e1c5c48 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sun, 27 Oct 2024 05:49:15 -0400 Subject: [PATCH 18/22] Remove obsolete JArrow Place machinery --- src/libraries/JANA/Topology/JArrow.h | 100 +-------------------- src/libraries/JANA/Topology/JFoldArrow.h | 5 +- src/libraries/JANA/Topology/JUnfoldArrow.h | 5 +- 3 files changed, 9 insertions(+), 101 deletions(-) diff --git a/src/libraries/JANA/Topology/JArrow.h b/src/libraries/JANA/Topology/JArrow.h index 7754a3dde..e2d44993e 100644 --- a/src/libraries/JANA/Topology/JArrow.h +++ b/src/libraries/JANA/Topology/JArrow.h @@ -13,10 +13,6 @@ #include -#ifndef JANA2_ARROWDATA_MAX_SIZE -#define JANA2_ARROWDATA_MAX_SIZE 10 -#endif - struct Place; using JEventQueue = JMailbox; @@ -94,114 +90,26 @@ class JArrow { void push(OutputData& outputs, size_t output_count, size_t location_id); }; -struct Data { - std::array items; - size_t item_count = 0; - size_t location_id; - - Data(size_t location_id = 0) : location_id(location_id) { - items = {nullptr}; - } -}; - struct Place { void* place_ref = nullptr; bool is_queue = true; bool is_input = false; - size_t min_item_count = 1; - size_t max_item_count = 1; Place(JArrow* parent, bool is_input) { assert(parent != nullptr); parent->attach(this); this->is_input = is_input; } - - Place(JArrow* parent, bool is_input, size_t min_item_count, size_t max_item_count) { - assert(parent != nullptr); - parent->attach(this); - this->is_input = is_input; - this->min_item_count = min_item_count; - this->max_item_count = max_item_count; - } - - void set_queue(JMailbox* queue) { - assert(queue != nullptr); - this->place_ref = queue; - this->is_queue = true; - } - - void set_pool(JEventPool* pool) { - assert(pool != nullptr); - this->place_ref = pool; - this->is_queue = false; - } - - size_t get_pending() { - assert(place_ref != nullptr); - if (is_input && is_queue) { - auto queue = static_cast*>(place_ref); - return queue->size(); - } - return 0; - } - - bool pull(Data& data) { - assert(place_ref != nullptr); - if (is_input) { // Actually pull the data - if (is_queue) { - auto queue = static_cast*>(place_ref); - data.item_count = queue->pop(data.items.data(), min_item_count, max_item_count, data.location_id); - return (data.item_count >= min_item_count); - } - else { - auto pool = static_cast(place_ref); - data.item_count = pool->pop(data.items.data(), min_item_count, max_item_count, data.location_id); - return (data.item_count >= min_item_count); - } - } - else { - data.item_count = 0; - return true; - } - } - - void revert(Data& data) { - assert(place_ref != nullptr); - if (is_queue) { - auto queue = static_cast*>(place_ref); - queue->push(data.items.data(), data.item_count, data.location_id); - } - else { - if (is_input) { - auto pool = static_cast(place_ref); - pool->push(data.items.data(), data.item_count, false, data.location_id); - } - } - } - - size_t push(Data& data) { - assert(place_ref != nullptr); - if (is_queue) { - auto queue = static_cast*>(place_ref); - queue->push(data.items.data(), data.item_count, data.location_id); - data.item_count = 0; - return is_input ? 0 : data.item_count; - } - else { - auto pool = static_cast(place_ref); - pool->push(data.items.data(), data.item_count, !is_input, data.location_id); - data.item_count = 0; - return 1; - } - } }; inline size_t JArrow::get_pending() { size_t sum = 0; for (Place* place : m_places) { - sum += place->get_pending(); + if (place->is_input && place->is_queue) { + auto queue = static_cast*>(place->place_ref); + sum += queue->size(); + } } return sum; } diff --git a/src/libraries/JANA/Topology/JFoldArrow.h b/src/libraries/JANA/Topology/JFoldArrow.h index 4bd12248b..c865c5b62 100644 --- a/src/libraries/JANA/Topology/JFoldArrow.h +++ b/src/libraries/JANA/Topology/JFoldArrow.h @@ -29,10 +29,7 @@ class JFoldArrow : public JTriggeredArrow { JEventLevel child_level) : m_parent_level(parent_level), - m_child_level(child_level), - m_child_in(this, true, 1, 1), - m_child_out(this, false, 1, 1), - m_parent_out(this, false, 1, 1) + m_child_level(child_level) { set_name(name); m_next_input_port = CHILD_IN; diff --git a/src/libraries/JANA/Topology/JUnfoldArrow.h b/src/libraries/JANA/Topology/JUnfoldArrow.h index aab5c4521..14db8d3e8 100644 --- a/src/libraries/JANA/Topology/JUnfoldArrow.h +++ b/src/libraries/JANA/Topology/JUnfoldArrow.h @@ -62,7 +62,10 @@ class JUnfoldArrow : public JTriggeredArrow { size_t get_pending() final { size_t sum = 0; for (Place* place : m_places) { - sum += place->get_pending(); + if (place->is_input && place->is_queue) { + auto queue = static_cast*>(place->place_ref); + sum += queue->size(); + } } if (m_parent_event != nullptr) { sum += 1; From bd8dc2d7eada4a987cd82e18cf30ac284b67e514 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sun, 27 Oct 2024 20:07:23 -0400 Subject: [PATCH 19/22] Fix JFoldArrow --- src/libraries/JANA/Topology/JFoldArrow.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/JANA/Topology/JFoldArrow.h b/src/libraries/JANA/Topology/JFoldArrow.h index c865c5b62..c7b9b193f 100644 --- a/src/libraries/JANA/Topology/JFoldArrow.h +++ b/src/libraries/JANA/Topology/JFoldArrow.h @@ -93,8 +93,10 @@ class JFoldArrow : public JTriggeredArrow { throw JException("JFoldArrow received a child with the wrong event level"); } - auto parent = const_cast(&event->GetParent(m_parent_level)); - m_folder->DoFold(*event, *parent); + if (m_folder != nullptr) { + auto parent = const_cast(&event->GetParent(m_parent_level)); + m_folder->DoFold(*event, *parent); + } status = JArrowMetrics::Status::KeepGoing; outputs[0] = {event, CHILD_OUT}; From 014a666b1bd0e64fc6c1e75fa09eda2df460348d Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sun, 27 Oct 2024 20:10:19 -0400 Subject: [PATCH 20/22] Replace JArrow::Place with JArrow::Port --- src/libraries/JANA/Topology/JArrow.cc | 65 ++++++++++++------- src/libraries/JANA/Topology/JArrow.h | 42 +++--------- src/libraries/JANA/Topology/JEventMapArrow.cc | 2 + src/libraries/JANA/Topology/JEventMapArrow.h | 3 - .../JANA/Topology/JEventSourceArrow.cc | 1 + .../JANA/Topology/JEventSourceArrow.h | 19 ++++-- src/libraries/JANA/Topology/JEventTapArrow.cc | 1 + src/libraries/JANA/Topology/JEventTapArrow.h | 2 - src/libraries/JANA/Topology/JFoldArrow.h | 25 ++++--- .../JANA/Topology/JTopologyBuilder.cc | 30 ++++----- src/libraries/JANA/Topology/JTriggeredArrow.h | 2 +- src/libraries/JANA/Topology/JUnfoldArrow.h | 29 ++++----- .../unit_tests/Topology/JArrowTests.cc | 4 +- .../Topology/TestTopologyComponents.h | 16 ++--- 14 files changed, 111 insertions(+), 130 deletions(-) diff --git a/src/libraries/JANA/Topology/JArrow.cc b/src/libraries/JANA/Topology/JArrow.cc index 18c8656f3..df636ce68 100644 --- a/src/libraries/JANA/Topology/JArrow.cc +++ b/src/libraries/JANA/Topology/JArrow.cc @@ -1,41 +1,50 @@ #include +void JArrow::create_ports(size_t inputs, size_t outputs) { + m_ports.clear(); + for (size_t i=0; i* queue, size_t port) { // Place index is relative to whether it is an input or not // Port index, however, is agnostic to whether it is an input or not - if (port >= m_places.size()) { - throw JException("Attempting to attach to a non-existent place! arrow=%s, port=%d", m_name.c_str(), port); + if (port >= m_ports.size()) { + throw JException("Attempting to attach to a non-existent port! arrow=%s, port=%d", m_name.c_str(), port); } - m_places[port]->is_queue = true; - m_places[port]->place_ref = queue; + m_ports[port].queue = queue; } void JArrow::attach(JEventPool* pool, size_t port) { // Place index is relative to whether it is an input or not // Port index, however, is agnostic to whether it is an input or not - if (port >= m_places.size()) { + if (port >= m_ports.size()) { throw JException("Attempting to attach to a non-existent place! arrow=%s, port=%d", m_name.c_str(), port); } - m_places[port]->is_queue = false; - m_places[port]->place_ref = pool; + m_ports[port].pool = pool; } -JEvent* JArrow::pull(size_t input_port, size_t location_id) { +JEvent* JArrow::pull(size_t port_index, size_t location_id) { JEvent* event = nullptr; - auto& place = m_places[input_port]; - if (place->is_queue) { - auto queue = static_cast*>(place->place_ref); - queue->pop(&event, 1, 1, location_id); + auto& port = m_ports.at(port_index); + if (port.queue != nullptr) { + port.queue->pop(&event, 1, 1, location_id); + } + else if (port.pool != nullptr){ + port.pool->pop(&event, 1, 1, location_id); } else { - auto pool = static_cast(place->place_ref); - pool->pop(&event, 1, 1, location_id); + throw JException("Arrow %s: Port %d not wired!", m_name.c_str(), port_index); } - // If ether pop() failed, the returned event is nullptr + // If pop() failed, the returned event is nullptr return event; } @@ -43,15 +52,27 @@ JEvent* JArrow::pull(size_t input_port, size_t location_id) { void JArrow::push(OutputData& outputs, size_t output_count, size_t location_id) { for (size_t output = 0; output < output_count; ++output) { JEvent* event = outputs[output].first; - int port = outputs[output].second; - if (m_places[port]->is_queue) { - auto queue = static_cast*>(m_places[port]->place_ref); - queue->push(&event, 1, location_id); + int port_index = outputs[output].second; + Port& port = m_ports.at(port_index); + if (port.queue != nullptr) { + port.queue->push(&event, 1, location_id); + } + else if (port.pool != nullptr) { + bool clear_event = !port.is_input; + port.pool->push(&event, 1, clear_event, location_id); } else { - auto pool = static_cast(m_places[port]->place_ref); - bool clear_event = !m_places[port]->is_input; - pool->push(&event, 1, clear_event, location_id); + throw JException("Arrow %s: Port %d not wired!", m_name.c_str(), port_index); + } + } +} + +inline size_t JArrow::get_pending() { + size_t sum = 0; + for (Port& port : m_ports) { + if (port.is_input && port.queue != nullptr) { + sum += port.queue->size(); } } + return sum; } diff --git a/src/libraries/JANA/Topology/JArrow.h b/src/libraries/JANA/Topology/JArrow.h index e2d44993e..2cea3f12f 100644 --- a/src/libraries/JANA/Topology/JArrow.h +++ b/src/libraries/JANA/Topology/JArrow.h @@ -13,10 +13,6 @@ #include -struct Place; -using JEventQueue = JMailbox; - - class JArrow { friend class JScheduler; friend class JTopologyBuilder; @@ -24,6 +20,12 @@ class JArrow { public: using OutputData = std::array, 2>; + struct Port { + JMailbox* queue = nullptr; + JEventPool* pool = nullptr; + bool is_input = false; + }; + private: std::string m_name; // Used for human understanding bool m_is_parallel; // Whether or not it is safe to parallelize @@ -31,11 +33,10 @@ class JArrow { bool m_is_sink; // Whether or not tnis arrow contributes to the final event count JArrowMetrics m_metrics; // Performance information accumulated over all workers - std::vector m_listeners; // Downstream Arrows - protected: + std::vector m_ports; // Will eventually supplant m_listeners + std::vector m_listeners; // Downstream Arrows JLogger m_logger; - std::vector m_places; // Will eventually supplant m_listeners public: std::string get_name() { return m_name; } @@ -77,11 +78,7 @@ class JArrow { m_listeners.push_back(downstream); }; - void attach(Place* place) { - if (std::find(m_places.begin(), m_places.end(), place) == m_places.end()) { - m_places.push_back(place); - } - }; + void create_ports(size_t inputs, size_t outputs); void attach(JMailbox* queue, size_t port); void attach(JEventPool* pool, size_t port); @@ -91,27 +88,6 @@ class JArrow { }; -struct Place { - void* place_ref = nullptr; - bool is_queue = true; - bool is_input = false; - - Place(JArrow* parent, bool is_input) { - assert(parent != nullptr); - parent->attach(this); - this->is_input = is_input; - } -}; -inline size_t JArrow::get_pending() { - size_t sum = 0; - for (Place* place : m_places) { - if (place->is_input && place->is_queue) { - auto queue = static_cast*>(place->place_ref); - sum += queue->size(); - } - } - return sum; -} diff --git a/src/libraries/JANA/Topology/JEventMapArrow.cc b/src/libraries/JANA/Topology/JEventMapArrow.cc index 6e971d2a7..feb9041e0 100644 --- a/src/libraries/JANA/Topology/JEventMapArrow.cc +++ b/src/libraries/JANA/Topology/JEventMapArrow.cc @@ -13,6 +13,8 @@ JEventMapArrow::JEventMapArrow(std::string name) { set_name(name); set_is_parallel(true); + create_ports(1, 1); + } void JEventMapArrow::add_source(JEventSource* source) { diff --git a/src/libraries/JANA/Topology/JEventMapArrow.h b/src/libraries/JANA/Topology/JEventMapArrow.h index e8d9e1230..346e1c7f5 100644 --- a/src/libraries/JANA/Topology/JEventMapArrow.h +++ b/src/libraries/JANA/Topology/JEventMapArrow.h @@ -15,9 +15,6 @@ class JEvent; class JEventMapArrow : public JTriggeredArrow { private: - Place m_input {this, true }; - Place m_output {this, false }; - std::vector m_sources; std::vector m_unfolders; std::vector m_procs; diff --git a/src/libraries/JANA/Topology/JEventSourceArrow.cc b/src/libraries/JANA/Topology/JEventSourceArrow.cc index de034a9a3..80b8a5db6 100644 --- a/src/libraries/JANA/Topology/JEventSourceArrow.cc +++ b/src/libraries/JANA/Topology/JEventSourceArrow.cc @@ -14,6 +14,7 @@ JEventSourceArrow::JEventSourceArrow(std::string name, std::vector { +public: + const size_t EVENT_IN = 0; + const size_t EVENT_OUT = 1; + private: std::vector m_sources; size_t m_current_source = 0; bool m_barrier_active = false; JEvent* m_pending_barrier_event = nullptr; - Place m_input {this, true}; - Place m_output {this, false}; - public: JEventSourceArrow(std::string name, std::vector sources); void set_input(JMailbox* queue) { - m_input.set_queue(queue); + m_ports[EVENT_IN].queue = queue; + m_ports[EVENT_IN].pool = nullptr; } void set_input(JEventPool* pool) { - m_input.set_pool(pool); + m_ports[EVENT_IN].queue = nullptr; + m_ports[EVENT_IN].pool = pool; } void set_output(JMailbox* queue) { - m_output.set_queue(queue); + m_ports[EVENT_OUT].queue = queue; + m_ports[EVENT_OUT].pool = nullptr; } void set_output(JEventPool* pool) { - m_output.set_pool(pool); + m_ports[EVENT_OUT].queue = nullptr; + m_ports[EVENT_OUT].pool = pool; } void initialize() final; diff --git a/src/libraries/JANA/Topology/JEventTapArrow.cc b/src/libraries/JANA/Topology/JEventTapArrow.cc index 67f1b89b1..13a4b3f3c 100644 --- a/src/libraries/JANA/Topology/JEventTapArrow.cc +++ b/src/libraries/JANA/Topology/JEventTapArrow.cc @@ -10,6 +10,7 @@ JEventTapArrow::JEventTapArrow(std::string name) { set_name(name); + create_ports(1,1); } void JEventTapArrow::add_processor(JEventProcessor* proc) { diff --git a/src/libraries/JANA/Topology/JEventTapArrow.h b/src/libraries/JANA/Topology/JEventTapArrow.h index ec5265b61..0e14cda80 100644 --- a/src/libraries/JANA/Topology/JEventTapArrow.h +++ b/src/libraries/JANA/Topology/JEventTapArrow.h @@ -13,8 +13,6 @@ class JEventTapArrow : public JTriggeredArrow { private: std::vector m_procs; - Place m_input {this, true }; - Place m_output {this, false }; public: JEventTapArrow(std::string name); diff --git a/src/libraries/JANA/Topology/JFoldArrow.h b/src/libraries/JANA/Topology/JFoldArrow.h index c7b9b193f..85a4de05e 100644 --- a/src/libraries/JANA/Topology/JFoldArrow.h +++ b/src/libraries/JANA/Topology/JFoldArrow.h @@ -18,10 +18,6 @@ class JFoldArrow : public JTriggeredArrow { JEventLevel m_parent_level; JEventLevel m_child_level; - Place m_child_in {this, true}; - Place m_child_out {this, false}; - Place m_parent_out {this, false}; - public: JFoldArrow( std::string name, @@ -32,6 +28,7 @@ class JFoldArrow : public JTriggeredArrow { m_child_level(child_level) { set_name(name); + create_ports(1, 2); m_next_input_port = CHILD_IN; } @@ -40,28 +37,28 @@ class JFoldArrow : public JTriggeredArrow { } void attach_child_in(JMailbox* child_in) { - m_child_in.place_ref = child_in; - m_child_in.is_queue = true; + m_ports[CHILD_IN].queue = child_in; + m_ports[CHILD_IN].pool = nullptr; } void attach_child_out(JMailbox* child_out) { - m_child_out.place_ref = child_out; - m_child_out.is_queue = true; + m_ports[CHILD_OUT].queue = child_out; + m_ports[CHILD_OUT].pool = nullptr; } void attach_child_out(JEventPool* child_out) { - m_child_out.place_ref = child_out; - m_child_out.is_queue = false; + m_ports[CHILD_OUT].queue = nullptr; + m_ports[CHILD_OUT].pool = child_out; } void attach_parent_out(JEventPool* parent_out) { - m_parent_out.place_ref = parent_out; - m_parent_out.is_queue = false; + m_ports[PARENT_OUT].queue = nullptr; + m_ports[PARENT_OUT].pool = parent_out; } void attach_parent_out(JMailbox* parent_out) { - m_parent_out.place_ref = parent_out; - m_parent_out.is_queue = true; + m_ports[PARENT_OUT].queue = parent_out; + m_ports[PARENT_OUT].pool = nullptr; } void initialize() final { diff --git a/src/libraries/JANA/Topology/JTopologyBuilder.cc b/src/libraries/JANA/Topology/JTopologyBuilder.cc index 29fa12c6c..b1dc6a01d 100644 --- a/src/libraries/JANA/Topology/JTopologyBuilder.cc +++ b/src/libraries/JANA/Topology/JTopologyBuilder.cc @@ -14,9 +14,6 @@ #include -using Event = std::shared_ptr; -using EventQueue = JMailbox; - JTopologyBuilder::JTopologyBuilder() { SetPrefix("jana"); } @@ -63,7 +60,7 @@ std::string JTopologyBuilder::print_topology() { for (JArrow* arrow : arrows) { show_row = true; - for (Place* place : arrow->m_places) { + for (JArrow::Port& port : arrow->m_ports) { if (show_row) { t | arrow->get_name(); t | arrow->is_parallel(); @@ -72,10 +69,11 @@ std::string JTopologyBuilder::print_topology() { else { t | "" | "" ; } + auto place_index = lookup[(port.queue!=nullptr) ? (void*) port.queue : (void*) port.pool]; - t | ((place->is_input) ? "Input ": "Output"); - t | ((place->is_queue) ? "Queue ": "Pool"); - t | lookup[place->place_ref]; + t | ((port.is_input) ? "Input ": "Output"); + t | ((port.queue != nullptr) ? "Queue ": "Pool"); + t | place_index; } } return t.Render(); @@ -148,26 +146,26 @@ void JTopologyBuilder::acquire_services(JServiceLocator *sl) { void JTopologyBuilder::connect(JArrow* upstream, size_t up_index, JArrow* downstream, size_t down_index) { - auto queue = new EventQueue(m_max_inflight_events, mapping.get_loc_count(), m_enable_stealing); + auto queue = new JMailbox(m_max_inflight_events, mapping.get_loc_count(), m_enable_stealing); queues.push_back(queue); size_t i = 0; - for (Place* place : upstream->m_places) { - if (!place->is_input) { + for (JArrow::Port& port : upstream->m_ports) { + if (!port.is_input) { if (i++ == up_index) { // Found the correct output - place->is_queue = true; - place->place_ref = queue; + port.queue = queue; + port.pool = nullptr; } } } i = 0; - for (Place* place : downstream->m_places) { - if (place->is_input) { + for (JArrow::Port& port : downstream->m_ports) { + if (port.is_input) { if (i++ == down_index) { // Found the correct input - place->is_queue = true; - place->place_ref = queue; + port.queue = queue; + port.pool = nullptr; } } } diff --git a/src/libraries/JANA/Topology/JTriggeredArrow.h b/src/libraries/JANA/Topology/JTriggeredArrow.h index c9ea10b6c..8cff29e62 100644 --- a/src/libraries/JANA/Topology/JTriggeredArrow.h +++ b/src/libraries/JANA/Topology/JTriggeredArrow.h @@ -54,7 +54,7 @@ struct JTriggeredArrow : public JArrow { // their input pool. These mustn't be counted as "processed" in the metrics. size_t processed_count = 0; for (size_t output=0; outputis_input) { + if (!m_ports[outputs[output].second].is_input) { processed_count++; } } diff --git a/src/libraries/JANA/Topology/JUnfoldArrow.h b/src/libraries/JANA/Topology/JUnfoldArrow.h index 14db8d3e8..3a64a4e0e 100644 --- a/src/libraries/JANA/Topology/JUnfoldArrow.h +++ b/src/libraries/JANA/Topology/JUnfoldArrow.h @@ -18,37 +18,33 @@ class JUnfoldArrow : public JTriggeredArrow { JEvent* m_parent_event = nullptr; JEvent* m_child_event = nullptr; - Place m_parent_in {this, true}; - Place m_child_in {this, true}; - Place m_child_out {this, false}; - public: JUnfoldArrow(std::string name, JEventUnfolder* unfolder) : m_unfolder(unfolder) { set_name(name); + create_ports(2, 1); m_next_input_port = PARENT_IN; } void attach_parent_in(JMailbox* parent_in) { - m_parent_in.place_ref = parent_in; - m_parent_in.is_queue = true; + m_ports[PARENT_IN].queue = parent_in; + m_ports[PARENT_IN].pool = nullptr; } void attach_child_in(JEventPool* child_in) { - m_child_in.place_ref = child_in; - m_child_in.is_queue = false; + m_ports[CHILD_IN].queue = nullptr; + m_ports[CHILD_IN].pool = child_in; } void attach_child_in(JMailbox* child_in) { - m_child_in.place_ref = child_in; - m_child_in.is_queue = true; + m_ports[CHILD_IN].queue = child_in; + m_ports[CHILD_IN].pool = nullptr; } void attach_child_out(JMailbox* child_out) { - m_child_out.place_ref = child_out; - m_child_out.is_queue = true; + m_ports[CHILD_OUT].queue = child_out; + m_ports[CHILD_OUT].pool = nullptr; } - void initialize() final { m_unfolder->DoInit(); LOG_INFO(m_logger) << "Initialized JEventUnfolder '" << m_unfolder->GetTypeName() << "'" << LOG_END; @@ -61,10 +57,9 @@ class JUnfoldArrow : public JTriggeredArrow { size_t get_pending() final { size_t sum = 0; - for (Place* place : m_places) { - if (place->is_input && place->is_queue) { - auto queue = static_cast*>(place->place_ref); - sum += queue->size(); + for (Port& port : m_ports) { + if (port.is_input && port.queue!=nullptr) { + sum += port.queue->size(); } } if (m_parent_event != nullptr) { diff --git a/src/programs/unit_tests/Topology/JArrowTests.cc b/src/programs/unit_tests/Topology/JArrowTests.cc index 6ea867112..42589a69a 100644 --- a/src/programs/unit_tests/Topology/JArrowTests.cc +++ b/src/programs/unit_tests/Topology/JArrowTests.cc @@ -8,10 +8,8 @@ struct TestData { int x; }; struct BasicParallelArrow : public JTriggeredArrow { - Place m_input {this, true }; - Place m_output {this, false }; - BasicParallelArrow() { + create_ports(1, 1); set_is_parallel(true); } diff --git a/src/programs/unit_tests/Topology/TestTopologyComponents.h b/src/programs/unit_tests/Topology/TestTopologyComponents.h index 0bf2b8734..4231ece3a 100644 --- a/src/programs/unit_tests/Topology/TestTopologyComponents.h +++ b/src/programs/unit_tests/Topology/TestTopologyComponents.h @@ -16,9 +16,6 @@ struct EventData { struct RandIntArrow : public JTriggeredArrow { - Place m_input {this, true }; - Place m_output {this, false }; - size_t emit_limit = 20; // How many to emit size_t emit_count = 0; // How many emitted so far int emit_sum = 0; // Sum of all ints emitted so far @@ -26,6 +23,7 @@ struct RandIntArrow : public JTriggeredArrow { RandIntArrow(std::string name, JEventPool* pool, JMailbox* output_queue) { set_name(name); set_is_source(true); + create_ports(1, 1); attach(pool, 0); attach(output_queue, 1); } @@ -57,12 +55,10 @@ struct RandIntArrow : public JTriggeredArrow { struct MultByTwoArrow : public JTriggeredArrow { - Place m_input {this, true }; - Place m_output {this, false }; - MultByTwoArrow(std::string name, JMailbox* input_queue, JMailbox* output_queue) { set_name(name); set_is_parallel(true); + create_ports(1, 1); attach(input_queue, 0); attach(output_queue, 1); } @@ -81,12 +77,10 @@ struct MultByTwoArrow : public JTriggeredArrow { struct SubOneArrow : public JTriggeredArrow { - Place m_input {this, true }; - Place m_output {this, false }; - SubOneArrow(std::string name, JMailbox* input_queue, JMailbox* output_queue) { set_name(name); set_is_parallel(true); + create_ports(1, 1); attach(input_queue, 0); attach(output_queue, 1); } @@ -107,14 +101,12 @@ struct SubOneArrow : public JTriggeredArrow { struct SumArrow : public JTriggeredArrow { - Place m_input {this, true }; - Place m_output {this, false }; - double sum = 0; SumArrow(std::string name, JMailbox* input_queue, JEventPool* pool) { set_name(name); set_is_sink(true); + create_ports(1, 1); attach(input_queue, 0); attach(pool, 1); } From 8e5d0971a3c9ce1bc33007b9fdc017e6a6012291 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sun, 27 Oct 2024 20:36:07 -0400 Subject: [PATCH 21/22] Make arrow attachment mechanism consistent --- src/libraries/JANA/Topology/JEventMapArrow.h | 3 ++ .../JANA/Topology/JEventSourceArrow.h | 20 +------------ src/libraries/JANA/Topology/JEventTapArrow.h | 2 ++ src/libraries/JANA/Topology/JFoldArrow.h | 29 +------------------ .../JANA/Topology/JTopologyBuilder.cc | 22 +++++++------- src/libraries/JANA/Topology/JUnfoldArrow.h | 24 +-------------- .../unit_tests/Components/UnfoldTests.cc | 12 ++++---- .../unit_tests/Topology/SubeventTests.cc | 4 +-- 8 files changed, 27 insertions(+), 89 deletions(-) diff --git a/src/libraries/JANA/Topology/JEventMapArrow.h b/src/libraries/JANA/Topology/JEventMapArrow.h index 346e1c7f5..5a63c8fe2 100644 --- a/src/libraries/JANA/Topology/JEventMapArrow.h +++ b/src/libraries/JANA/Topology/JEventMapArrow.h @@ -14,6 +14,9 @@ class JEvent; class JEventMapArrow : public JTriggeredArrow { +public: + enum PortIndex {EVENT_IN=0, EVENT_OUT=1}; + private: std::vector m_sources; std::vector m_unfolders; diff --git a/src/libraries/JANA/Topology/JEventSourceArrow.h b/src/libraries/JANA/Topology/JEventSourceArrow.h index cdb8aa877..419652ad0 100644 --- a/src/libraries/JANA/Topology/JEventSourceArrow.h +++ b/src/libraries/JANA/Topology/JEventSourceArrow.h @@ -8,8 +8,7 @@ class JEventSourceArrow : public JTriggeredArrow { public: - const size_t EVENT_IN = 0; - const size_t EVENT_OUT = 1; + enum PortIndex {EVENT_IN=0, EVENT_OUT=1}; private: std::vector m_sources; @@ -20,23 +19,6 @@ class JEventSourceArrow : public JTriggeredArrow { public: JEventSourceArrow(std::string name, std::vector sources); - void set_input(JMailbox* queue) { - m_ports[EVENT_IN].queue = queue; - m_ports[EVENT_IN].pool = nullptr; - } - void set_input(JEventPool* pool) { - m_ports[EVENT_IN].queue = nullptr; - m_ports[EVENT_IN].pool = pool; - } - void set_output(JMailbox* queue) { - m_ports[EVENT_OUT].queue = queue; - m_ports[EVENT_OUT].pool = nullptr; - } - void set_output(JEventPool* pool) { - m_ports[EVENT_OUT].queue = nullptr; - m_ports[EVENT_OUT].pool = pool; - } - void initialize() final; void finalize() final; void fire(JEvent* input, OutputData& outputs, size_t& output_count, JArrowMetrics::Status& status); diff --git a/src/libraries/JANA/Topology/JEventTapArrow.h b/src/libraries/JANA/Topology/JEventTapArrow.h index 0e14cda80..ee9b59206 100644 --- a/src/libraries/JANA/Topology/JEventTapArrow.h +++ b/src/libraries/JANA/Topology/JEventTapArrow.h @@ -10,6 +10,8 @@ class JEvent; class JEventTapArrow : public JTriggeredArrow { +public: + enum PortIndex {EVENT_IN=0, EVENT_OUT=1}; private: std::vector m_procs; diff --git a/src/libraries/JANA/Topology/JFoldArrow.h b/src/libraries/JANA/Topology/JFoldArrow.h index 85a4de05e..270d740b8 100644 --- a/src/libraries/JANA/Topology/JFoldArrow.h +++ b/src/libraries/JANA/Topology/JFoldArrow.h @@ -8,9 +8,7 @@ class JFoldArrow : public JTriggeredArrow { public: - const int CHILD_IN = 0; - const int CHILD_OUT = 1; - const int PARENT_OUT = 2; + enum PortIndex {CHILD_IN=0, CHILD_OUT=1, PARENT_OUT=2}; private: JEventFolder* m_folder = nullptr; @@ -36,31 +34,6 @@ class JFoldArrow : public JTriggeredArrow { m_folder = folder; } - void attach_child_in(JMailbox* child_in) { - m_ports[CHILD_IN].queue = child_in; - m_ports[CHILD_IN].pool = nullptr; - } - - void attach_child_out(JMailbox* child_out) { - m_ports[CHILD_OUT].queue = child_out; - m_ports[CHILD_OUT].pool = nullptr; - } - - void attach_child_out(JEventPool* child_out) { - m_ports[CHILD_OUT].queue = nullptr; - m_ports[CHILD_OUT].pool = child_out; - } - - void attach_parent_out(JEventPool* parent_out) { - m_ports[PARENT_OUT].queue = nullptr; - m_ports[PARENT_OUT].pool = parent_out; - } - - void attach_parent_out(JMailbox* parent_out) { - m_ports[PARENT_OUT].queue = parent_out; - m_ports[PARENT_OUT].pool = nullptr; - } - void initialize() final { if (m_folder != nullptr) { m_folder->DoInit(); diff --git a/src/libraries/JANA/Topology/JTopologyBuilder.cc b/src/libraries/JANA/Topology/JTopologyBuilder.cc index b1dc6a01d..cc986b62b 100644 --- a/src/libraries/JANA/Topology/JTopologyBuilder.cc +++ b/src/libraries/JANA/Topology/JTopologyBuilder.cc @@ -259,8 +259,8 @@ void JTopologyBuilder::attach_level(JEventLevel current_level, JUnfoldArrow* par bool need_source = !sources_at_level.empty(); if (need_source) { src_arrow = new JEventSourceArrow(level_str+"Source", sources_at_level); - src_arrow->set_input(pool_at_level); - src_arrow->set_output(pool_at_level); + src_arrow->attach(pool_at_level, JEventSourceArrow::EVENT_IN); + src_arrow->attach(pool_at_level, JEventSourceArrow::EVENT_OUT); arrows.push_back(src_arrow); } @@ -285,8 +285,8 @@ void JTopologyBuilder::attach_level(JEventLevel current_level, JUnfoldArrow* par for (JEventUnfolder* unf: unfolders_at_level) { map1_arrow->add_unfolder(unf); } - map1_arrow->attach(pool_at_level, 0); - map1_arrow->attach(pool_at_level, 1); + map1_arrow->attach(pool_at_level, JEventMapArrow::EVENT_IN); + map1_arrow->attach(pool_at_level, JEventMapArrow::EVENT_OUT); arrows.push_back(map1_arrow); } @@ -308,7 +308,7 @@ void JTopologyBuilder::attach_level(JEventLevel current_level, JUnfoldArrow* par if(need_fold) { fold_arrow = new JFoldArrow(level_str+"Fold", current_level, unfolders_at_level[0]->GetChildLevel()); arrows.push_back(fold_arrow); - fold_arrow->attach_parent_out(pool_at_level); + fold_arrow->attach(pool_at_level, JFoldArrow::PARENT_OUT); } // -------------------------- @@ -320,8 +320,8 @@ void JTopologyBuilder::attach_level(JEventLevel current_level, JUnfoldArrow* par map2_arrow = new JEventMapArrow(level_str+"Map2"); for (JEventProcessor* proc : mappable_procs_at_level) { map2_arrow->add_processor(proc); - map2_arrow->attach(pool_at_level, 0); - map2_arrow->attach(pool_at_level, 1); + map2_arrow->attach(pool_at_level, JEventMapArrow::EVENT_IN); + map2_arrow->attach(pool_at_level, JEventMapArrow::EVENT_OUT); } arrows.push_back(map2_arrow); } @@ -335,8 +335,8 @@ void JTopologyBuilder::attach_level(JEventLevel current_level, JUnfoldArrow* par tap_arrow = new JEventTapArrow(level_str+"Tap"); for (JEventProcessor* proc : tappable_procs_at_level) { tap_arrow->add_processor(proc); - tap_arrow->attach(pool_at_level, 0); - tap_arrow->attach(pool_at_level, 1); + tap_arrow->attach(pool_at_level, JEventTapArrow::EVENT_IN); + tap_arrow->attach(pool_at_level, JEventTapArrow::EVENT_OUT); } arrows.push_back(tap_arrow); } @@ -347,7 +347,7 @@ void JTopologyBuilder::attach_level(JEventLevel current_level, JUnfoldArrow* par // 1. Source // -------------------------- if (parent_unfolder != nullptr) { - parent_unfolder->attach_child_in(pool_at_level); + parent_unfolder->attach(pool_at_level, JUnfoldArrow::CHILD_IN); connect_to_first_available(parent_unfolder, {map1_arrow, unfold_arrow, map2_arrow, tap_arrow, parent_folder}); } if (src_arrow != nullptr) { @@ -366,7 +366,7 @@ void JTopologyBuilder::attach_level(JEventLevel current_level, JUnfoldArrow* par connect_to_first_available(tap_arrow, {parent_folder}); } if (parent_folder != nullptr) { - parent_folder->attach_child_out(pool_at_level); + parent_folder->attach(pool_at_level, JFoldArrow::CHILD_OUT); } // Finally, we recur over lower levels! diff --git a/src/libraries/JANA/Topology/JUnfoldArrow.h b/src/libraries/JANA/Topology/JUnfoldArrow.h index 3a64a4e0e..24ac72cb9 100644 --- a/src/libraries/JANA/Topology/JUnfoldArrow.h +++ b/src/libraries/JANA/Topology/JUnfoldArrow.h @@ -9,9 +9,7 @@ class JUnfoldArrow : public JTriggeredArrow { public: - const int PARENT_IN = 0; - const int CHILD_IN = 1; - const int CHILD_OUT = 2; + enum PortIndex {PARENT_IN=0, CHILD_IN=1, CHILD_OUT=2}; private: JEventUnfolder* m_unfolder = nullptr; @@ -25,26 +23,6 @@ class JUnfoldArrow : public JTriggeredArrow { m_next_input_port = PARENT_IN; } - void attach_parent_in(JMailbox* parent_in) { - m_ports[PARENT_IN].queue = parent_in; - m_ports[PARENT_IN].pool = nullptr; - } - - void attach_child_in(JEventPool* child_in) { - m_ports[CHILD_IN].queue = nullptr; - m_ports[CHILD_IN].pool = child_in; - } - - void attach_child_in(JMailbox* child_in) { - m_ports[CHILD_IN].queue = child_in; - m_ports[CHILD_IN].pool = nullptr; - } - - void attach_child_out(JMailbox* child_out) { - m_ports[CHILD_OUT].queue = child_out; - m_ports[CHILD_OUT].pool = nullptr; - } - void initialize() final { m_unfolder->DoInit(); LOG_INFO(m_logger) << "Initialized JEventUnfolder '" << m_unfolder->GetTypeName() << "'" << LOG_END; diff --git a/src/programs/unit_tests/Components/UnfoldTests.cc b/src/programs/unit_tests/Components/UnfoldTests.cc index cdb048af0..b73297ea9 100644 --- a/src/programs/unit_tests/Components/UnfoldTests.cc +++ b/src/programs/unit_tests/Components/UnfoldTests.cc @@ -60,9 +60,9 @@ TEST_CASE("UnfoldTests_Basic") { TestUnfolder unfolder; JUnfoldArrow arrow("sut", &unfolder); - arrow.attach_parent_in(&parent_queue); - arrow.attach_child_in(&child_pool); - arrow.attach_child_out(&child_queue); + arrow.attach(&parent_queue, JUnfoldArrow::PARENT_IN); + arrow.attach(&child_pool, JUnfoldArrow::CHILD_IN); + arrow.attach(&child_queue, JUnfoldArrow::CHILD_OUT); JArrowMetrics m; arrow.initialize(); @@ -97,9 +97,9 @@ TEST_CASE("FoldArrowTests") { JMailbox parent_out; JFoldArrow arrow("sut", JEventLevel::Timeslice, JEventLevel::PhysicsEvent); - arrow.attach_child_in(&child_in); - arrow.attach_child_out(&child_out); - arrow.attach_parent_out(&parent_out); + arrow.attach(&child_in, JFoldArrow::CHILD_IN); + arrow.attach(&child_out, JFoldArrow::CHILD_OUT); + arrow.attach(&parent_out, JFoldArrow::PARENT_OUT); JArrowMetrics metrics; arrow.initialize(); diff --git a/src/programs/unit_tests/Topology/SubeventTests.cc b/src/programs/unit_tests/Topology/SubeventTests.cc index 4883107e3..6a654314b 100644 --- a/src/programs/unit_tests/Topology/SubeventTests.cc +++ b/src/programs/unit_tests/Topology/SubeventTests.cc @@ -103,8 +103,8 @@ TEST_CASE("Basic subevent arrow functionality") { topology->set_configure_fn([&](JTopologyBuilder& topology) { auto source_arrow = new JEventSourceArrow("simpleSource", {new SimpleSource}); - source_arrow->set_input(topology.event_pool); - source_arrow->set_output(&events_in); + source_arrow->attach(topology.event_pool, JEventSourceArrow::EVENT_IN); + source_arrow->attach(&events_in, JEventSourceArrow::EVENT_OUT); auto proc_arrow = new JEventMapArrow("simpleProcessor"); proc_arrow->attach(&events_out, 0); From 79e48a995378c99b50312091d87cb3b44a6f5d35 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Tue, 29 Oct 2024 14:18:18 -0400 Subject: [PATCH 22/22] Tiny fix --- src/libraries/JANA/Topology/JArrow.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/JANA/Topology/JArrow.cc b/src/libraries/JANA/Topology/JArrow.cc index df636ce68..2c58cec76 100644 --- a/src/libraries/JANA/Topology/JArrow.cc +++ b/src/libraries/JANA/Topology/JArrow.cc @@ -67,7 +67,7 @@ void JArrow::push(OutputData& outputs, size_t output_count, size_t location_id) } } -inline size_t JArrow::get_pending() { +size_t JArrow::get_pending() { size_t sum = 0; for (Port& port : m_ports) { if (port.is_input && port.queue != nullptr) {