fix the execution order of entites in executors to favor insertion order #2537

wjwwood · 2024-05-23T00:38:38Z

In specific circumstances, multiple entities (of the same kind) in an executor can be ready at the same time and the executor must arbitrarily decide which to execute first, and in Iron and before, this fell back to the order in which they were added, but in Jazzy we (inadvertently) changed this and it became related to the hash order, subtly changing the order of execution that some folks had come to rely on, see: #2532

While this ordering was not guaranteed, this pr aims to restore the previous behavior for consistency and to provide a less surprising result, but the order is still not a guarantee nor is it the same for all executor types (e.g. the EventsExecutor is consistent, but is instead based on the trigger order of the entities, not their insertion/addition order).

The new test is also in a separate pr here: #2536 so it can be more easily back ported to previous versions. I think it should pass on them, but I still need to test it. See that pr for updates.

This approach will not work for backporting as it breaks ABI, but I believe I could make a version that doesn't break ABI (it will be as ugly as past non-ABI breaking changes). I just want to make sure we want to follow through with this before committing resources to that attempt.

This pr does incur some additional overhead, but I believe it should be relatively low, considering what we're storing. It does, however, slow down insertion and erasure from the collections which could be a concern in the tight loop of spin. I will do some more experimentation related to this, but the memory overhead of storing the entity pointers of the collection twice (once as an unordered map for fast random access and once in a vector for maintaining insertion order) should be relatively small. It does increase the memory footprint by 2x, but the footprint was small to begin with (three pointers per entity).

I looked at using existing "ordered maps" but all of them (that I could see) either provided the desired API but lost the fast lookup of the hash or did what I did here and maintained the information twice in two different data structures.

So I think this is a reasonable approach, but I'm up for discussion on alternatives.

Signed-off-by: William Woodall <[email protected]>

wjwwood · 2024-05-23T00:39:44Z

rclcpp/include/rclcpp/executors/executor_entities_collection.hpp

-  for (auto it = update_to.begin(); it != update_to.end(); ) {
+  for (auto it = update_to.begin_ordered(); it != update_to.end_ordered(); ) {
    if (update_from.count(it->first) == 0) {
      auto entity = it->second.entity.lock();
      if (entity) {
        on_removed(entity);
      }
-      it = update_to.erase(it);
+      it = update_to.erase_ordered(it);
    } else {
      ++it;
    }
  }
-  for (auto it = update_from.begin(); it != update_from.end(); ++it) {
+  for (auto it = update_from.begin_ordered(); it != update_from.end_ordered(); ++it) {
    if (update_to.count(it->first) == 0) {
      auto entity = it->second.entity.lock();
      if (entity) {
        on_added(entity);
      }
-      update_to.insert(*it);
+      bool inserted = update_to.insert(*it);
+      // Should never be false, so this is a defensive check, mark unused too
+      // in order to avoid a warning in release builds.
+      assert(inserted);
+      RCUTILS_UNUSED(inserted);


This is the important part of the changes that affect the order of execution when tied, because this is what controls the order in which they are added to the wait set in the case of the wait set based executors, i.e. everything except the EventsExecutor.

wjwwood · 2024-05-23T00:41:43Z

rclcpp/include/rclcpp/executors/executor_entities_collection.hpp

+  /// Type of the map used for random access
+  using MapType = std::unordered_map<const EntityKeyType *, CollectionEntry<EntityValueType>>;
+
+  /// Type of the vector for insertion order access
+  // Note, we cannot use typename MapType::value_type because it makes the first
+  // item in the pair const, which prevents copy assignment of the pair, which
+  // prevents std::vector::erase from working later...
+  using VectorType = std::vector<std::pair<const EntityKeyType *,
+      CollectionEntry<EntityValueType>>>;


This detail was annoying, the MapType::value_type makes the first argument of the pair const (would be const EntityKeyType * const in this case), which prevents the use of the operator= on the pair, which prevents std::vector::erase from working...

Anyway, this is essentially a std::vector<typename MapType::value_type> except for that detail...

Why not just store the iterator returned by std::map::insert ?

Because they can be invalidated by future insertions.

I also considered storing the pointer (which is the key for the map) only, but then when iterating in order you need to do a lookup in the map for each one you want to use, which is made redundant if you just store the information twice. In fact this conclusion is what several of the "ordered dict in c++" projects I looked at online ended up using.

Because they can be invalidated by future insertions.

Nope, not for std::map :
https://en.cppreference.com/w/cpp/container/map/insert

No iterators or references are invalidated. If the insertion is successful, pointers and references to the element obtained while it is held in the node handle are invalidated, and pointers and references obtained to that element before it was extracted become valid.(since C++17)

But the point is to use the unordered_map so that you get the better look up performance (Constant on average, worst case linear in the size of the container. for unordered_map vs Logarithmic in the size of the container. for map), and in that case I interpret it to mean that rehashing will invalidate them:

If after the operation the new number of elements is greater than old max_load_factor() * bucket_count() a rehashing takes place.
If rehashing occurs (due to the insertion), all iterators are invalidated. Otherwise (no rehashing), iterators are not invalidated. If the insertion is successful, pointers and references to the element obtained while it is held in the node handle are invalidated, and pointers and references obtained to that element before it was extracted become valid.(since C++17)

-- https://en.cppreference.com/w/cpp/container/unordered_map/insert

Which is also the conclusion of this SO answer (which I was using to try and confirm my interpretation): https://stackoverflow.com/a/54004916

For what it's worth, I considered using std::map and its iterators too.
However, the conclusion I came to is that whether it's a vector of iterators (pointers) or a vector of triplets of pointers, the impact is not much different on memory, even at relatively large scales. Instead, I thought preserving the cpu time optimizations was better for the executor, given the feedback we've consistently gotten, but that's a generalization.

wjwwood · 2024-05-23T00:45:01Z

rclcpp/src/rclcpp/executors/executor_entities_collection.cpp

  auto remove_entities = [](auto & collection) -> size_t {
      size_t removed = 0;
-      for (auto it = collection.begin(); it != collection.end(); ) {
+      for (auto it = collection.begin_ordered(); it != collection.end_ordered(); ) {
        if (it->second.entity.expired()) {
          ++removed;
-          it = collection.erase(it);
+          it = collection.erase_ordered(it);


This change was not strictly necessary, but I reasoned it was slightly faster to provide the iterator for erasure to the vector and look up the iterator for the map, rather than the other way around, which is what erase_ordered does.

wjwwood · 2024-05-23T00:47:49Z

rclcpp/src/rclcpp/experimental/executors/events_executor/events_executor.cpp

-  collection.insert(
+  bool inserted = collection.insert(
  {
    this->notify_waitable_.get(),
    {this->notify_waitable_, weak_group_ptr}
  });
+  // Explicitly ignore if the notify waitable was not inserted because that means
+  // it was already inserted, which happens initially as it is explicitly added
+  // in the constructor as well as every time the collection is reset, so on
+  // the first reset there is a second insertion attempt.
+  // We could check before trying to insert, but that would require a "find" call
+  // on each refresh, which is expensive, and otherwise it would require additional
+  // state in this class to detect the initial case where it is added twice.
+  // Therefore we just insert and ignore it if it fails (the only way it fails
+  // is when a duplicate is inserted).
+  RCUTILS_UNUSED(inserted);


This is an oddity I found with the events executor, where it was always inserting the notify waitable twice on one occasion (first refresh after construction), and it wasn't checking the return value of insert, which if we had checked would have occasionally been false meaning it wasn't inserted. This didn't end up mattering, but it did feel weird and broke some things when I added the vector storage along side the map.

Anyway I landed on this approach where we continue to add it twice sometimes, but we just ignore it when it wasn't inserted as the fact that it has been inserted is important, not the order in which it was, at least in the specific case of this notify waitable.

FYI @mauropasse

I think that this was a bug.
I opened a PR to fix it #2564

Cool, thanks for checking up on it. I guess I'll wait for that pr to merge and then update this. I have to check into timer execution order on Windows for these new tests anyways.

rclcpp/src/rclcpp/executor.cpp

rclcpp/include/rclcpp/executors/executor_entities_collection.hpp

thomasmoore-torc · 2024-05-23T21:32:06Z

As I've been looking into how all of this works, it seems to me that the bulk of the complexity associated with the ExecutorEntitiesCollection is to allow the Executor class to handle callback groups. So far as I can tell, the ExecutorEntitiesCollection essentially serves two purposes:

To act as a staging area for entities to be added to the waitset
To associate a callback group with an entity

As the SingleThreadedExecutor only needs the first purpose and the StaticSingleThreadedExecutor needs neither, would it be worth considering the "don't pay for what you don't use" principal and refactoring some of the purpose-specific functionality out of the base Executor class and either directly into the executor implementations that require that functionality or into intermediate implementations that can be used by multiple executors as needed?

wjwwood · 2024-05-23T22:26:05Z

As the SingleThreadedExecutor only needs the first purpose and the StaticSingleThreadedExecutor needs neither

I don't think that's actually true. The SingleThreadedExecutor does actually consider the callback groups, but in a very limited fashion. There's actually a test that disables individual callback groups explicitly, and checks that executors do not execute from them. Now whether or not that's useful isn't clear to me, but as it stands CallbackGroups are a non-optional part of the executor related APIs, and for better worse we cannot just ignore them. Now we could change that, but it would require changing some of the semantics, but that's not something we can do in a stable release, in my opinion. But we can consider it for the future.

From a technical angle, there's nothing that requires that any executor, including the two you mentioned, use the Entity Collection classes, it was just convenient. However, in my experience you cannot remove much if anything that's there and still handle ownership mechanics correctly, e.g. nodes hold pointers to callback groups which holds pointers to entities and you have to handle gracefully removing them when the user lets any go out of scope, but you have to hold ownership in the executor while using them, etc... So feel free to try and optimize them, but just be aware that there be dragons, lol.

Also, despite the name, the StaticSingleThreadedExecutor does actually need to do both 1 and 2 that you listed. And in fact after the recent refactoring there's little to no practical difference between it and the SingleThreadedExecutor, we just didn't manage to get it deprecated in the chaos of getting everything else done before the API freeze this time. (someone disagree with me if you think otherwise)

Signed-off-by: William Woodall <[email protected]>

alsora · 2024-05-29T15:34:20Z

As the SingleThreadedExecutor only needs the first purpose and the StaticSingleThreadedExecutor needs neither

As discussed in the client library WG, an important feature (that is currently achieved through callback groups) is to allow to add only some of the node's entities to an executor

This pr does incur some additional overhead, but I believe it should be relatively low, considering what we're storing. It does, however, slow down insertion and erasure from the collections which could be a concern in the tight loop of spin

We usually decided to not focus too much on the overhead of insertion/deletion (compared to the overhead of the fast-loop: selection and execution of entities).
Most of the ROS 2 systems will setup their nodes and then just run with a static graph,

Signed-off-by: William Woodall <[email protected]>

wjwwood · 2024-05-30T01:16:04Z

We usually decided to not focus too much on the overhead of insertion/deletion (compared to the overhead of the fast-loop: selection and execution of entities).
Most of the ROS 2 systems will setup their nodes and then just run with a static graph,

I'm not sure if we only ever insert/delete when the user adds or removes something, e.g. copying or updating a collection can happen even when the user hasn't added or removed anything, but maybe I'm wrong about that. We have two collections in the executor base class which are swapped (sort of) at specific intervals:

rclcpp/rclcpp/src/rclcpp/executor.cpp

Lines 683 to 717 in 3bc364a

    
           // Update each of the groups of entities in the current collection, adding or removing 
        
           // from the wait set as necessary. 
        
           current_collection_.timers.update( 
        
             collection.timers, 
        
             [this](auto timer) {wait_set_.add_timer(timer);}, 
        
             [this](auto timer) {wait_set_.remove_timer(timer);}); 
        
           current_collection_.subscriptions.update( 
        
             collection.subscriptions, 
        
             [this](auto subscription) { 
        
               wait_set_.add_subscription(subscription, kDefaultSubscriptionMask); 
        
             }, 
        
             [this](auto subscription) { 
        
               wait_set_.remove_subscription(subscription, kDefaultSubscriptionMask); 
        
             }); 
        
           current_collection_.clients.update( 
        
             collection.clients, 
        
             [this](auto client) {wait_set_.add_client(client);}, 
        
             [this](auto client) {wait_set_.remove_client(client);}); 
        
           current_collection_.services.update( 
        
             collection.services, 
        
             [this](auto service) {wait_set_.add_service(service);}, 
        
             [this](auto service) {wait_set_.remove_service(service);}); 
        
           current_collection_.guard_conditions.update( 
        
             collection.guard_conditions, 
        
             [this](auto guard_condition) {wait_set_.add_guard_condition(guard_condition);}, 
        
             [this](auto guard_condition) {wait_set_.remove_guard_condition(guard_condition);}); 
        
           current_collection_.waitables.update( 
        
             collection.waitables, 
        
             [this](auto waitable) {wait_set_.add_waitable(waitable);}, 
        
             [this](auto waitable) {wait_set_.remove_waitable(waitable);});

Anyway, maybe @mjcarroll can expand on that.

This is still something I'm going to investigate before pushing to merge this.

alsora · 2024-05-30T02:33:23Z

Well, rebuilding the collection only when something changes was the purpose of the "notifier waitable" and the "entities collection" classes, and that's how they were used in the events executor and static single threaded executor.
Unless we did something wrong while expanding them to the single threaded executor, i would expect the same behavior.

Signed-off-by: William Woodall <[email protected]>

wjwwood · 2024-06-13T14:15:18Z

Well, rebuilding the collection only when something changes was the purpose of the "notifier waitable" and the "entities collection" classes, and that's how they were used in the events executor and static single threaded executor.
Unless we did something wrong while expanding them to the single threaded executor, i would expect the same behavior.

That was my hope, but I wanted to confirm which I am going to do soon hopefully.

mjcarroll · 2024-06-14T15:48:37Z

Unless we did something wrong while expanding them to the single threaded executor, i would expect the same behavior.

That is the behavior unless something was done incorrectly.

If the set of the entities in the collection stays constant, the waitset should not be rebuilt.

wjwwood · 2024-06-18T23:57:44Z

I closed #2536 in favor of this pr (it was just the test, with no fix), but cross-posting here for visibility since there were some good discussions on that pr too.

alsora · 2024-06-19T17:18:28Z

@wjwwood did you have a chance to check if the collection was correctly functioning? i.e. it's rebuilt only when something changes.
P.S. this is something we should be able to easy unit-test.

fujitatomoya

overall lgtm, just a minor comment for the test.

fujitatomoya · 2024-06-21T07:20:37Z

rclcpp/test/rclcpp/executors/test_executors.cpp

+      "reverse call order",
+      {
+        {"waitable", {false, reverse, forward}},
+        {"subscription", {false, reverse, forward}},


should we skip this test, i think the same test is executed twice.

Suggested change

{"subscription", {false, reverse, forward}},

{"subscription", {true, reverse, forward}},

Can you explain?

I looked again, it doesn't seem like we're doing the same test twice (not exactly), so maybe you could explain why you think that?

If we were to disable this test, then we should do {true, {},{}} also, as the call order vs expected order don't matter if it is skipped.

i think call_order is not used in the test of the subscription (maybe cannot be used), so it always check against expected_execution_order which is forward. in that case, we are doing the same test twice?

wjwwood · 2024-06-25T22:41:43Z

I can confirm that the insertions only occur when entities are created or destroyed, so the overhead of this fix should be minimal.

I just added print statements during insertion as well as some to show what steps of the test we're in:

FINDME >> start executor test, before executor creation
FINDME >> before adding callback group
insert(0xac797083e690) insert(0xac7970886c00) insert(0xac79708aeb30) insert(0xac79708c2780) insert(0xac79708c1970) insert(0xac79708d45d0) insert(0xac79708fcbe0) insert(0xac79708fbd30) insert(0xac79708fc410) insert(0xac7970934f10) insert(0xac7970947cf0) insert(0xac797095e870) insert(0xac7970972220) insert(0xac79709706a0) insert(0xac7970997dd0) insert(0xac7970996ca0) insert(0xac79709a9c00) insert(0xac79709bcc50) insert(0xac79709e3910) insert(0xac79709f6800) insert(0xac797089b4a0) insert(0xac797089b300) insert(0xac79708ae480) insert(0xac7970629a60) insert(0xac79708c2050) insert(0xac7970629820) insert(0xac79708d4cc0) insert(0xac79706295e0) insert(0xac79708e7840) insert(0xac7970629310) insert(0xac79708fc570) insert(0xac79708fc440) insert(0xac797090f270) insert(0xac797090f140) insert(0xac7970921d30) insert(0xac79708fc220) insert(0xac7970934770) insert(0xac7970934640) insert(0xac7970947660) insert(0xac7970628770) insert(0xac797095e280) insert(0xac79706284a0) insert(0xac7970971c30) insert(0xac79706267e0) insert(0xac79709847d0) insert(0xac79706265e0) insert(0xac7970997740) insert(0xac79708adb90) insert(0xac79709aa780) insert(0xac79709aa5c0) insert(0xac79709bd650) insert(0xac79709bd490) insert(0xac79709d05b0) insert(0xac79709d03f0) insert(0xac79709e3280) insert(0xac79709e30c0) insert(0xac79709f6170) insert(0xac79709f5fb0) insert(0xac7970a09170) insert(0xac7970a08fb0) insert(0xac79707ff8a0) insert(0xac797083e690) insert(0xac7970886c00) insert(0xac79708aeb30) insert(0xac79708c2780) insert(0xac79708c1970) insert(0xac79708d45d0) insert(0xac79708fcbe0) insert(0xac79708fbd30) insert(0xac79708fc410) insert(0xac7970934f10) insert(0xac7970947cf0) insert(0xac797095e870) insert(0xac7970972220) insert(0xac79709706a0) insert(0xac7970997dd0) insert(0xac7970996ca0) insert(0xac79709a9c00) insert(0xac79709bcc50) insert(0xac79709e3910) insert(0xac79709f6800) insert(0xac797089b4a0) insert(0xac797089b300) insert(0xac79708ae480) insert(0xac7970629a60) insert(0xac79708c2050) insert(0xac7970629820) insert(0xac79708d4cc0) insert(0xac79706295e0) insert(0xac79708e7840) insert(0xac7970629310) insert(0xac79708fc570) insert(0xac79708fc440) insert(0xac797090f270) insert(0xac797090f140) insert(0xac7970921d30) insert(0xac79708fc220) insert(0xac7970934770) insert(0xac7970934640) insert(0xac7970947660) insert(0xac7970628770) insert(0xac797095e280) insert(0xac79706284a0) insert(0xac7970971c30) insert(0xac79706267e0) insert(0xac79709847d0) insert(0xac79706265e0) insert(0xac7970997740) insert(0xac79708adb90) insert(0xac79709aa780) insert(0xac79709aa5c0) insert(0xac79709bd650) insert(0xac79709bd490) insert(0xac79709d05b0) insert(0xac79709d03f0) insert(0xac79709e3280) insert(0xac79709e30c0) insert(0xac79709f6170) insert(0xac79709f5fb0) insert(0xac7970a09170) insert(0xac7970a08fb0) insert(0xac79707ff8a0) FINDME >> spinning to complete executor test: 0 / 20
FINDME >> spinning to complete executor test: 1 / 20
FINDME >> spinning to complete executor test: 2 / 20
FINDME >> spinning to complete executor test: 3 / 20
FINDME >> spinning to complete executor test: 4 / 20
FINDME >> spinning to complete executor test: 5 / 20
FINDME >> spinning to complete executor test: 6 / 20
FINDME >> spinning to complete executor test: 7 / 20
FINDME >> spinning to complete executor test: 8 / 20
FINDME >> spinning to complete executor test: 9 / 20
FINDME >> spinning to complete executor test: 10 / 20
FINDME >> spinning to complete executor test: 11 / 20
FINDME >> spinning to complete executor test: 12 / 20
FINDME >> spinning to complete executor test: 13 / 20
FINDME >> spinning to complete executor test: 14 / 20
FINDME >> spinning to complete executor test: 15 / 20
FINDME >> spinning to complete executor test: 16 / 20
FINDME >> spinning to complete executor test: 17 / 20
FINDME >> spinning to complete executor test: 18 / 20
FINDME >> spinning to complete executor test: 19 / 20
FINDME >> stop executor test

You can see that it does some insertions when the callback group is added, but not between spins, which was expected.

Signed-off-by: William Woodall <[email protected]>

wjwwood · 2024-06-25T22:53:03Z

P.S. this is something we should be able to easy unit-test.

I tested it with print statements, and I started to add a unit test, but checking it as a unit test became pretty difficult, since it's not just testing a unit, but rather two working together (with different results for each pair), e.g. STE with the EntityCollection vs the MTE with the EntityCollection vs the EventsExecutor with the EntityCollection. I tried to add a test that just used some methods of the EntityCollection, but it's not set up for introspection, so I kind of ran out of time to make the changes needed to properly test this. Probably with a derived class only used in the test or perhaps some hooks that can be installed just for testing.

So unfortunately I'll have to leave that for others or future me to investigate.

jmachowinski · 2024-06-26T11:59:34Z

rclcpp/include/rclcpp/executors/executor_entities_collection.hpp

    if (update_to.count(it->first) == 0) {
      auto entity = it->second.entity.lock();
      if (entity) {
        on_added(entity);
      }
-      update_to.insert(*it);
+      bool inserted = update_to.insert(*it);


Why insert an outdated entity? I guess this should be in the if(entity) ?

jmachowinski · 2024-06-26T12:08:36Z

rclcpp/include/rclcpp/executors/executor_entities_collection.hpp

@@ -72,34 +73,47 @@ void update_entities(
  std::function<void(const typename CollectionType::EntitySharedPtr &)> on_removed
 )
 {


I think here is a potential bug. If between two call of update_entities the order of elements in update_from changes, the order in update_to will not change. I am not really sure if this might happen though.

wjwwood added 2 commits May 22, 2024 17:02

add test to check for execution order of entities in various executors

82ce8fb

Signed-off-by: William Woodall <[email protected]>

change how we iterate over entity collections to prefer insertion order

d096060

Signed-off-by: William Woodall <[email protected]>

wjwwood added the bug Something isn't working label May 23, 2024

wjwwood self-assigned this May 23, 2024

wjwwood requested review from ivanpauno and hidmic as code owners May 23, 2024 00:38

wjwwood commented May 23, 2024

View reviewed changes

This was referenced May 23, 2024

add test to check for execution order of entities in various executors #2536

Closed

Executor callbacks are no longer in a predictable order #2532

Open

thomasmoore-torc reviewed May 23, 2024

View reviewed changes

rclcpp/src/rclcpp/executor.cpp Show resolved Hide resolved

rclcpp/include/rclcpp/executors/executor_entities_collection.hpp Show resolved Hide resolved

add tests for subscriptions in the execution order

2b9bbc9

Signed-off-by: William Woodall <[email protected]>

refactor test case structure and add cases for timers

2f16ecf

Signed-off-by: William Woodall <[email protected]>

add service cases for execution order ub test

5dd08da

Signed-off-by: William Woodall <[email protected]>

wjwwood mentioned this pull request Jun 12, 2024

[iron] add test to check for execution order of entities in various executors #2561

Closed

alsora mentioned this pull request Jun 19, 2024

avoid adding notify waitable twice to events-executor collection #2564

Merged

fujitatomoya reviewed Jun 21, 2024

View reviewed changes

fixups

a5ea0da

Signed-off-by: William Woodall <[email protected]>

jmachowinski reviewed Jun 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix the execution order of entites in executors to favor insertion order #2537

fix the execution order of entites in executors to favor insertion order #2537

wjwwood commented May 23, 2024

wjwwood May 23, 2024

wjwwood May 23, 2024

jmachowinski May 23, 2024

wjwwood May 23, 2024

wjwwood May 23, 2024

jmachowinski May 24, 2024

wjwwood May 28, 2024

wjwwood May 23, 2024

wjwwood May 23, 2024

alsora May 29, 2024

alsora Jun 19, 2024

wjwwood Jun 25, 2024

thomasmoore-torc commented May 23, 2024

wjwwood commented May 23, 2024 •

edited

Loading

alsora commented May 29, 2024

wjwwood commented May 30, 2024

alsora commented May 30, 2024

wjwwood commented Jun 13, 2024

mjcarroll commented Jun 14, 2024

wjwwood commented Jun 18, 2024

alsora commented Jun 19, 2024

fujitatomoya left a comment

fujitatomoya Jun 21, 2024

wjwwood Jun 25, 2024

wjwwood Jun 25, 2024

fujitatomoya Jun 25, 2024

wjwwood commented Jun 25, 2024

wjwwood commented Jun 25, 2024

jmachowinski Jun 26, 2024 •

edited

Loading

jmachowinski Jun 26, 2024

	{"subscription", {false, reverse, forward}},
	{"subscription", {true, reverse, forward}},

fix the execution order of entites in executors to favor insertion order #2537

Are you sure you want to change the base?

fix the execution order of entites in executors to favor insertion order #2537

Conversation

wjwwood commented May 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomasmoore-torc commented May 23, 2024

wjwwood commented May 23, 2024 • edited Loading

alsora commented May 29, 2024

wjwwood commented May 30, 2024

alsora commented May 30, 2024

wjwwood commented Jun 13, 2024

mjcarroll commented Jun 14, 2024

wjwwood commented Jun 18, 2024

alsora commented Jun 19, 2024

fujitatomoya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjwwood commented Jun 25, 2024

wjwwood commented Jun 25, 2024

jmachowinski Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjwwood commented May 23, 2024 •

edited

Loading

jmachowinski Jun 26, 2024 •

edited

Loading