From b2bb4f139a7cf06a12a63f96e7a0bfb611aad735 Mon Sep 17 00:00:00 2001 From: Mike Bauer Date: Wed, 1 Dec 2021 15:40:00 -0800 Subject: [PATCH 1/9] legion: more work on refactoring distributed expressions and tracking references --- runtime/legion/legion_analysis.cc | 44 ++++++----- runtime/legion/legion_analysis.h | 6 +- runtime/legion/legion_instances.cc | 18 +++-- runtime/legion/legion_trace.cc | 16 ++-- runtime/legion/legion_views.cc | 15 +++- runtime/legion/legion_views.h | 3 + runtime/legion/region_tree.cc | 119 ++++++++++++++++++++--------- runtime/legion/region_tree.h | 65 ++++++++++------ runtime/legion/region_tree.inl | 24 +++--- 9 files changed, 199 insertions(+), 111 deletions(-) diff --git a/runtime/legion/legion_analysis.cc b/runtime/legion/legion_analysis.cc index e77f2401f5..7d2e0953ce 100644 --- a/runtime/legion/legion_analysis.cc +++ b/runtime/legion/legion_analysis.cc @@ -80,7 +80,7 @@ namespace Legion { #ifdef DEBUG_LEGION assert(expr != NULL); #endif - expr->add_expression_reference(); + expr->add_base_expression_reference(IS_EXPR_REF); } #else //-------------------------------------------------------------------------- @@ -92,7 +92,7 @@ namespace Legion { #ifdef DEBUG_LEGION assert(expr != NULL); #endif - expr->add_expression_reference(); + expr->add_base_expression_reference(IS_EXPR_REF); } #endif @@ -116,7 +116,7 @@ namespace Legion { #ifdef DEBUG_LEGION assert(expr != NULL); #endif - if (expr->remove_expression_reference()) + if (expr->remove_base_expression_reference(IS_EXPR_REF)) delete expr; } @@ -8666,7 +8666,7 @@ namespace Legion { pending_analyses(0) //-------------------------------------------------------------------------- { - set_expr->add_expression_reference(); + set_expr->add_nested_expression_reference(did); if (index_space_node != NULL) { #ifdef DEBUG_LEGION @@ -8702,7 +8702,7 @@ namespace Legion { EquivalenceSet::~EquivalenceSet(void) //-------------------------------------------------------------------------- { - if (set_expr->remove_expression_reference()) + if (set_expr->remove_nested_expression_reference(did)) delete set_expr; if ((index_space_node != NULL) && index_space_node->remove_nested_resource_ref(did)) @@ -8754,7 +8754,7 @@ namespace Legion { for (FieldMaskSet::const_iterator it = unrefined_remainders.begin(); it != unrefined_remainders.end(); it++) - if (it->first->remove_expression_reference()) + if (it->first->remove_nested_expression_reference(did)) delete it->first; } if (subset_exprs != NULL) @@ -9028,7 +9028,7 @@ namespace Legion { to_delete.begin(); it != to_delete.end(); it++) { unrefined_remainders.erase(*it); - if ((*it)->remove_expression_reference()) + if ((*it)->remove_nested_expression_reference(did)) delete (*it); } unrefined_remainders.tighten_valid_mask(); @@ -9266,7 +9266,7 @@ namespace Legion { assert(unrefined_remainders.get_valid_mask() * overlap); #endif if (unrefined_remainders.insert(diff_expr, overlap)) - diff_expr->add_expression_reference(); + diff_expr->add_nested_expression_reference(did); } } // Remove these fields from the overlap indicating @@ -9455,7 +9455,7 @@ namespace Legion { it->set_mask); #endif if (unrefined_remainders.insert(remainder, it->set_mask)) - remainder->add_expression_reference(); + remainder->add_nested_expression_reference(did); } } } @@ -9499,7 +9499,7 @@ namespace Legion { to_filter); #endif if (unrefined_remainders.insert(remainder, to_filter)) - remainder->add_expression_reference(); + remainder->add_nested_expression_reference(did); } } } @@ -9596,7 +9596,7 @@ namespace Legion { forest->subtract_index_spaces(set_expr, expr); #endif if (unrefined_remainders.insert(diff, ray_mask)) - diff->add_expression_reference(); + diff->add_nested_expression_reference(did); ray_mask.clear(); } } @@ -10245,7 +10245,7 @@ namespace Legion { unrefined_remainders.clear(); // Defer removing the references on these expressions until // the migration has been done - DeferRemoveRefArgs args(references); + DeferRemoveRefArgs args(references, did); runtime->issue_runtime_meta_task(args, LG_THROUGHPUT_WORK_PRIORITY, done_migration); } @@ -10426,7 +10426,7 @@ namespace Legion { FieldMask mask; derez.deserialize(mask); if (unrefined_remainders.insert(expr, mask)) - expr->add_expression_reference(); + expr->add_nested_expression_reference(did); } size_t num_disjoint_refinements; derez.deserialize(num_disjoint_refinements); @@ -13395,7 +13395,7 @@ namespace Legion { assert(unrefined_remainders.get_valid_mask() * finalize_mask); #endif if (unrefined_remainders.insert(diff_expr, finalize_mask)) - diff_expr->add_expression_reference(); + diff_expr->add_nested_expression_reference(did); } } @@ -13438,7 +13438,7 @@ namespace Legion { it = to_delete.begin(); it != to_delete.end(); it++) { unrefined_remainders.erase(*it); - if ((*it)->remove_expression_reference()) + if ((*it)->remove_nested_expression_reference(did)) delete (*it); } unrefined_remainders.tighten_valid_mask(); @@ -13448,7 +13448,7 @@ namespace Legion { for (FieldMaskSet::const_iterator it = to_add.begin(); it != to_add.end(); it++) if (unrefined_remainders.insert(it->first, it->second)) - it->first->add_expression_reference(); + it->first->add_nested_expression_reference(did); } } @@ -13865,7 +13865,7 @@ namespace Legion { //-------------------------------------------------------------------------- { if (local) - expr->add_expression_reference(); + expr->add_base_expression_reference(IS_EXPR_REF); } //-------------------------------------------------------------------------- @@ -13885,7 +13885,8 @@ namespace Legion { // Clean up our ray mask delete dargs->ray_mask; // Remove our expression reference too - if (dargs->is_local && dargs->expr->remove_expression_reference()) + if (dargs->is_local && + dargs->expr->remove_base_expression_reference(IS_EXPR_REF)) delete dargs->expr; } @@ -13982,7 +13983,7 @@ namespace Legion { //-------------------------------------------------------------------------- { if (is_local) - expr->add_expression_reference(); + expr->add_base_expression_reference(IS_EXPR_REF); } //-------------------------------------------------------------------------- @@ -14072,7 +14073,8 @@ namespace Legion { // Once construction is complete then we do the registration set->register_with_runtime(NULL/*no remote registration needed*/); // Remove our expression reference too - if (dargs->is_local && dargs->expr->remove_expression_reference()) + if (dargs->is_local && + dargs->expr->remove_base_expression_reference(IS_EXPR_REF)) delete dargs->expr; } @@ -14083,7 +14085,7 @@ namespace Legion { const DeferRemoveRefArgs *dargs = (const DeferRemoveRefArgs*)args; for (std::vector::const_iterator it = dargs->references->begin(); it != dargs->references->end(); it++) - if ((*it)->remove_expression_reference()) + if ((*it)->remove_nested_expression_reference(dargs->source)) delete (*it); delete dargs->references; } diff --git a/runtime/legion/legion_analysis.h b/runtime/legion/legion_analysis.h index 8bdf470e1d..01681648bc 100644 --- a/runtime/legion/legion_analysis.h +++ b/runtime/legion/legion_analysis.h @@ -2231,11 +2231,13 @@ namespace Legion { public: static const LgTaskID TASK_ID = LG_DEFER_REMOVE_EQ_REF_TASK_ID; public: - DeferRemoveRefArgs(std::vector *refs) + DeferRemoveRefArgs(std::vector *refs, + DistributedID src) : LgTaskArgs(implicit_provenance), - references(refs) { } + references(refs), source(src) { } public: std::vector *const references; + const DistributedID source; }; protected: enum EqState { diff --git a/runtime/legion/legion_instances.cc b/runtime/legion/legion_instances.cc index 525c7b7a17..cbf204d642 100644 --- a/runtime/legion/legion_instances.cc +++ b/runtime/legion/legion_instances.cc @@ -548,9 +548,9 @@ namespace Legion { if (layout != NULL) layout->add_reference(); if (field_space_node != NULL) - field_space_node->add_base_gc_ref(PHYSICAL_MANAGER_REF); + field_space_node->add_nested_gc_ref(did); if (instance_domain != NULL) - instance_domain->add_expression_reference(); + instance_domain->add_nested_expression_reference(did); } //-------------------------------------------------------------------------- @@ -560,10 +560,10 @@ namespace Legion { if ((layout != NULL) && layout->remove_reference()) delete layout; if ((field_space_node != NULL) && - field_space_node->remove_base_gc_ref(PHYSICAL_MANAGER_REF)) + field_space_node->remove_nested_gc_ref(did)) delete field_space_node; if ((instance_domain != NULL) && - instance_domain->remove_expression_reference()) + instance_domain->remove_nested_expression_reference(did)) delete instance_domain; } @@ -1592,7 +1592,7 @@ namespace Legion { //-------------------------------------------------------------------------- { if (local_is) - local_expr->add_expression_reference(); + local_expr->add_base_expression_reference(IS_EXPR_REF); } //-------------------------------------------------------------------------- @@ -1613,7 +1613,8 @@ namespace Legion { dargs->piece_list_size, space_node, dargs->tree_id, constraints, dargs->use_event, dargs->redop, dargs->shadow_instance); // Remove the local expression reference if necessary - if (dargs->local_is && dargs->local_expr->remove_expression_reference()) + if (dargs->local_is && + dargs->local_expr->remove_base_expression_reference(IS_EXPR_REF)) delete dargs->local_expr; } @@ -3021,7 +3022,7 @@ namespace Legion { //-------------------------------------------------------------------------- { if (local_is) - local_expr->add_expression_reference(); + local_expr->add_base_expression_reference(IS_EXPR_REF); } //-------------------------------------------------------------------------- @@ -3044,7 +3045,8 @@ namespace Legion { dargs->piece_list_size, space_node, dargs->tree_id, constraints, dargs->use_event, dargs->redop); // Remove the local expression reference if necessary - if (dargs->local_is && dargs->local_expr->remove_expression_reference()) + if (dargs->local_is && + dargs->local_expr->remove_base_expression_reference(IS_EXPR_REF)) delete dargs->local_expr; } diff --git a/runtime/legion/legion_trace.cc b/runtime/legion/legion_trace.cc index 0d37521738..ce3cc507b3 100644 --- a/runtime/legion/legion_trace.cc +++ b/runtime/legion/legion_trace.cc @@ -5548,14 +5548,14 @@ namespace Legion { assert(precondition_idx < tpl.events.size()); assert(expr != NULL); #endif - expr->add_expression_reference(); + expr->add_base_expression_reference(TRACE_REF); } //-------------------------------------------------------------------------- IssueCopy::~IssueCopy(void) //-------------------------------------------------------------------------- { - if (expr->remove_expression_reference()) + if (expr->remove_base_expression_reference(TRACE_REF)) delete expr; } @@ -5646,7 +5646,7 @@ namespace Legion { assert(precondition_idx < tpl.events.size()); assert(expr != NULL); #endif - expr->add_expression_reference(); + expr->add_base_expression_reference(TRACE_REF); indirections.resize(indirects.size()); for (unsigned idx = 0; idx < indirects.size(); idx++) indirections[idx] = indirects[idx]->clone(); @@ -5656,7 +5656,7 @@ namespace Legion { IssueIndirect::~IssueIndirect(void) //-------------------------------------------------------------------------- { - if (expr->remove_expression_reference()) + if (expr->remove_base_expression_reference(TRACE_REF)) delete expr; for (unsigned idx = 0; idx < indirections.size(); idx++) delete indirections[idx]; @@ -5747,7 +5747,7 @@ namespace Legion { assert(precondition_idx < tpl.events.size()); assert(expr != NULL); #endif - expr->add_expression_reference(); + expr->add_base_expression_reference(TRACE_REF); src->add_base_resource_ref(TRACE_REF); dst->add_base_resource_ref(TRACE_REF); } @@ -5756,7 +5756,7 @@ namespace Legion { GPUReduction::~GPUReduction(void) //-------------------------------------------------------------------------- { - if (expr->remove_expression_reference()) + if (expr->remove_base_expression_reference(TRACE_REF)) delete expr; if (src->remove_base_resource_ref(TRACE_REF)) delete src; @@ -5846,7 +5846,7 @@ namespace Legion { assert(fields.size() > 0); assert(precondition_idx < tpl.events.size()); #endif - expr->add_expression_reference(); + expr->add_base_expression_reference(TRACE_REF); fill_value = malloc(fill_size); memcpy(fill_value, value, fill_size); } @@ -5855,7 +5855,7 @@ namespace Legion { IssueFill::~IssueFill(void) //-------------------------------------------------------------------------- { - if (expr->remove_expression_reference()) + if (expr->remove_base_expression_reference(TRACE_REF)) delete expr; free(fill_value); } diff --git a/runtime/legion/legion_views.cc b/runtime/legion/legion_views.cc index 13daf44135..dcb15c3493 100644 --- a/runtime/legion/legion_views.cc +++ b/runtime/legion/legion_views.cc @@ -749,16 +749,19 @@ namespace Legion { InstanceView *view, IndexSpaceExpression *exp) : context(ctx), manager(man), inst_view(view), view_expr(exp), view_volume(view_expr->get_volume()), +#if defined(DEBUG_LEGION_GC) || defined(LEGION_GC) + view_did(view->did), +#endif invalid_fields(FieldMask(LEGION_FIELD_MASK_FIELD_ALL_ONES)) //-------------------------------------------------------------------------- { - view_expr->add_expression_reference(); + view_expr->add_nested_expression_reference(view->did); } //-------------------------------------------------------------------------- ExprView::ExprView(const ExprView &rhs) : context(rhs.context), manager(rhs.manager), inst_view(rhs.inst_view), - view_expr(rhs.view_expr), view_volume(rhs.view_volume) + view_did(0), view_expr(rhs.view_expr), view_volume(rhs.view_volume) //-------------------------------------------------------------------------- { // should never be called @@ -769,8 +772,14 @@ namespace Legion { ExprView::~ExprView(void) //-------------------------------------------------------------------------- { - if (view_expr->remove_expression_reference()) +#if defined(DEBUG_LEGION_GC) || defined(LEGION_GC) + if (view_expr->remove_nested_expression_reference(view_did)) + delete view_expr; +#else + // We can lie about the did here since its not actually used + if (view_expr->remove_nested_expression_reference(0/*bogus did*/)) delete view_expr; +#endif if (!subviews.empty()) { for (FieldMaskSet::iterator it = subviews.begin(); diff --git a/runtime/legion/legion_views.h b/runtime/legion/legion_views.h index 6186c35701..d8c6e5fb35 100644 --- a/runtime/legion/legion_views.h +++ b/runtime/legion/legion_views.h @@ -443,6 +443,9 @@ namespace Legion { InstanceView *const inst_view; IndexSpaceExpression *const view_expr; const size_t view_volume; +#if defined(DEBUG_LEGION_GC) || defined(LEGION_GC) + const DistributedID view_did; +#endif // This is publicly mutable and protected by expr_lock from // the owner inst_view FieldMask invalid_fields; diff --git a/runtime/legion/region_tree.cc b/runtime/legion/region_tree.cc index eaef5703bd..999de75122 100644 --- a/runtime/legion/region_tree.cc +++ b/runtime/legion/region_tree.cc @@ -6342,7 +6342,6 @@ namespace Legion { IndexSpaceExprID remote_expr_id; derez.deserialize(remote_expr_id); IndexSpaceExpression *result = unpack_expression_value(derez, source); - result->add_expression_reference(); { AutoLock l_lock(lookup_is_op_lock); #ifdef DEBUG_LEGION @@ -6427,7 +6426,7 @@ namespace Legion { { const TightenIndexSpaceArgs *targs = (const TightenIndexSpaceArgs*)args; targs->proxy_this->tighten_index_space(); - if (targs->proxy_this->remove_expression_tree_reference()) + if (targs->proxy_this->remove_base_expression_reference(IS_EXPR_REF)) delete targs->proxy_this; } @@ -6481,7 +6480,7 @@ namespace Legion { // forest has given us a reference back on it, see if we're the first // ones to write it, if not we can remove the reference now if (!__sync_bool_compare_and_swap(&canonical, NULL, expr)) - expr->remove_expression_tree_reference(); + expr->remove_canonical_reference(); return expr; } @@ -6577,7 +6576,7 @@ namespace Legion { //-------------------------------------------------------------------------- { // We always keep a reference on ourself until we get invalidated - add_expression_tree_reference(); + add_base_resource_ref(IS_EXPR_REF); #ifdef LEGION_GC log_garbage.info("GC Index Expr %lld %d %lld", LEGION_DISTRIBUTED_ID_FILTER(this->did), local_space, expr_id); @@ -6651,47 +6650,72 @@ namespace Legion { } //-------------------------------------------------------------------------- - void IndexSpaceOperation::add_expression_reference( - std::set &applied_events, unsigned count) + void IndexSpaceOperation::add_base_expression_reference( + ReferenceSource source, ReferenceMutator *mutator, unsigned count) + //-------------------------------------------------------------------------- + { + if (mutator == NULL) + { + LocalReferenceMutator local_mutator; + add_base_gc_ref(source, &local_mutator, count); + } + else + add_base_gc_ref(source, mutator, count); + } + + //-------------------------------------------------------------------------- + void IndexSpaceOperation::add_nested_expression_reference( + DistributedID source, std::set &applied_events, unsigned count) //-------------------------------------------------------------------------- { WrapperReferenceMutator mutator(applied_events); - add_expression_reference(&mutator, count); + add_nested_expression_reference(source, &mutator, count); } //-------------------------------------------------------------------------- - void IndexSpaceOperation::add_expression_reference( - ReferenceMutator *mutator, unsigned count) + void IndexSpaceOperation::add_nested_expression_reference( + DistributedID source, ReferenceMutator *mutator, unsigned count) //-------------------------------------------------------------------------- { if (mutator == NULL) { LocalReferenceMutator local_mutator; - add_base_gc_ref(IS_EXPR_REF, &local_mutator, count); + add_nested_gc_ref(source, &local_mutator, count); } else - add_base_gc_ref(IS_EXPR_REF, mutator, count); + add_nested_gc_ref(source, mutator, count); + } + + //-------------------------------------------------------------------------- + bool IndexSpaceOperation::remove_base_expression_reference( + ReferenceSource source, unsigned count) + //-------------------------------------------------------------------------- + { + return remove_base_gc_ref(source, NULL/*mutator*/, count); } //-------------------------------------------------------------------------- - bool IndexSpaceOperation::remove_expression_reference(unsigned count) + bool IndexSpaceOperation::remove_nested_expression_reference( + DistributedID source, unsigned count) //-------------------------------------------------------------------------- { - return remove_base_gc_ref(IS_EXPR_REF, NULL/*mutator*/, count); + return remove_nested_gc_ref(source, NULL/*mutator*/, count); } //-------------------------------------------------------------------------- - void IndexSpaceOperation::add_expression_tree_reference(unsigned count) + void IndexSpaceOperation::add_tree_expression_reference(DistributedID id, + unsigned count) //-------------------------------------------------------------------------- { - add_base_resource_ref(IS_EXPR_REF, count); + add_nested_resource_ref(id, count); } //-------------------------------------------------------------------------- - bool IndexSpaceOperation::remove_expression_tree_reference(unsigned count) + bool IndexSpaceOperation::remove_tree_expression_reference(DistributedID id, + unsigned count) //-------------------------------------------------------------------------- { - return remove_base_resource_ref(IS_EXPR_REF, count); + return remove_nested_resource_ref(id, count); } //-------------------------------------------------------------------------- @@ -6722,7 +6746,7 @@ namespace Legion { { // Add a reference to prevent the parents from being collected // as we're traversing up the tree - (*it)->add_base_resource_ref(IS_EXPR_REF); + (*it)->add_tree_expression_reference(did); parents[idx] = (*it); } } @@ -6733,7 +6757,7 @@ namespace Legion { { (*it)->invalidate_operation(to_remove); // Remove the reference when we're done with the parents - if ((*it)->remove_base_resource_ref(IS_EXPR_REF)) + if ((*it)->remove_tree_expression_reference(did)) delete (*it); } } @@ -7482,7 +7506,7 @@ namespace Legion { parent_operations.begin(); it != parent_operations.end(); it++, idx++) { - (*it)->add_expression_tree_reference(); + (*it)->add_tree_expression_reference(did); parents[idx] = (*it); } } @@ -7493,7 +7517,7 @@ namespace Legion { // Remove any references that we have on the parents for (std::vector::const_iterator it = parents.begin(); it != parents.end(); it++) - if ((*it)->remove_expression_tree_reference()) + if ((*it)->remove_tree_expression_reference(did)) delete (*it); } } @@ -8669,54 +8693,79 @@ namespace Legion { } //-------------------------------------------------------------------------- - void IndexSpaceNode::add_expression_reference( - std::set &applied_events, unsigned count) + void IndexSpaceNode::add_base_expression_reference( + ReferenceSource source, ReferenceMutator *mutator, unsigned count) + //-------------------------------------------------------------------------- + { + if (mutator == NULL) + { + LocalReferenceMutator local_mutator; + add_base_gc_ref(source, &local_mutator, count); + } + else + add_base_gc_ref(source, mutator, count); + } + + //-------------------------------------------------------------------------- + void IndexSpaceNode::add_nested_expression_reference( + DistributedID source, std::set &applied_events, unsigned count) //-------------------------------------------------------------------------- { WrapperReferenceMutator mutator(applied_events); - add_expression_reference(&mutator, count); + add_nested_expression_reference(source, &mutator, count); } //-------------------------------------------------------------------------- - void IndexSpaceNode::add_expression_reference( - ReferenceMutator *mutator, unsigned count) + void IndexSpaceNode::add_nested_expression_reference( + DistributedID source, ReferenceMutator *mutator, unsigned count) //-------------------------------------------------------------------------- { if (mutator == NULL) { LocalReferenceMutator local_mutator; - add_base_gc_ref(IS_EXPR_REF, &local_mutator, count); + add_nested_gc_ref(source, &local_mutator, count); } else - add_base_gc_ref(IS_EXPR_REF, mutator, count); + add_nested_gc_ref(source, mutator, count); + } + + //-------------------------------------------------------------------------- + bool IndexSpaceNode::remove_base_expression_reference( + ReferenceSource source, unsigned count) + //-------------------------------------------------------------------------- + { + return remove_base_gc_ref(source, NULL/*mutator*/, count); } //-------------------------------------------------------------------------- - bool IndexSpaceNode::remove_expression_reference(unsigned count) + bool IndexSpaceNode::remove_nested_expression_reference( + DistributedID source, unsigned count) //-------------------------------------------------------------------------- { - return remove_base_gc_ref(IS_EXPR_REF, NULL/*mutator*/, count); + return remove_nested_gc_ref(source, NULL/*mutator*/, count); } //-------------------------------------------------------------------------- - void IndexSpaceNode::add_expression_tree_reference(unsigned count) + void IndexSpaceNode::add_tree_expression_reference(DistributedID id, + unsigned count) //-------------------------------------------------------------------------- { - add_base_resource_ref(IS_EXPR_REF, count); + add_nested_resource_ref(id, count); } //-------------------------------------------------------------------------- - bool IndexSpaceNode::remove_expression_tree_reference(unsigned count) + bool IndexSpaceNode::remove_tree_expression_reference(DistributedID id, + unsigned count) //-------------------------------------------------------------------------- { - return remove_base_resource_ref(IS_EXPR_REF, count); + return remove_nested_resource_ref(id, count); } //-------------------------------------------------------------------------- bool IndexSpaceNode::remove_operation(RegionTreeForest *forest) //-------------------------------------------------------------------------- { - return remove_expression_tree_reference(); + return remove_base_resource_ref(IS_EXPR_REF); } //-------------------------------------------------------------------------- diff --git a/runtime/legion/region_tree.h b/runtime/legion/region_tree.h index 2abce33a6f..79ee089030 100644 --- a/runtime/legion/region_tree.h +++ b/runtime/legion/region_tree.h @@ -1028,7 +1028,7 @@ namespace Legion { TightenIndexSpaceArgs(IndexSpaceExpression *proxy) : LgTaskArgs(implicit_provenance), proxy_this(proxy) - { proxy->add_expression_tree_reference(); } + { proxy->add_base_expression_reference(IS_EXPR_REF); } public: IndexSpaceExpression *const proxy_this; }; @@ -1094,13 +1094,20 @@ namespace Legion { public: virtual bool try_add_canonical_reference(void) = 0; virtual bool remove_canonical_reference(void) = 0; - virtual void add_expression_reference(std::set &applied_events, - unsigned count = 1) = 0; - virtual void add_expression_reference(ReferenceMutator *mutator = NULL, - unsigned count = 1) = 0; - virtual bool remove_expression_reference(unsigned count = 1) = 0; - virtual void add_expression_tree_reference(unsigned count = 1) = 0; - virtual bool remove_expression_tree_reference(unsigned count = 1) = 0; + virtual void add_base_expression_reference(ReferenceSource source, + ReferenceMutator *mutator = NULL, unsigned count = 1) = 0; + virtual void add_nested_expression_reference(DistributedID source, + std::set &applied_events, unsigned count = 1) = 0; + virtual void add_nested_expression_reference(DistributedID source, + ReferenceMutator *mutator = NULL, unsigned count = 1) = 0; + virtual bool remove_base_expression_reference(ReferenceSource source, + unsigned count = 1) = 0; + virtual bool remove_nested_expression_reference(DistributedID source, + unsigned count = 1) = 0; + virtual void add_tree_expression_reference(DistributedID source, + unsigned count = 1) = 0; + virtual bool remove_tree_expression_reference(DistributedID source, + unsigned count = 1) = 0; public: virtual bool remove_operation(RegionTreeForest *forest) = 0; virtual IndexSpaceNode* create_node(IndexSpace handle, @@ -1348,13 +1355,20 @@ namespace Legion { public: virtual bool try_add_canonical_reference(void); virtual bool remove_canonical_reference(void); - virtual void add_expression_reference(std::set &applied_events, - unsigned count = 1); - virtual void add_expression_reference(ReferenceMutator *mutator = NULL, - unsigned count = 1); - virtual bool remove_expression_reference(unsigned count = 1); - virtual void add_expression_tree_reference(unsigned count = 1); - virtual bool remove_expression_tree_reference(unsigned count = 1); + virtual void add_base_expression_reference(ReferenceSource source, + ReferenceMutator *mutator = NULL, unsigned count = 1); + virtual void add_nested_expression_reference(DistributedID source, + std::set &applied_events, unsigned count = 1); + virtual void add_nested_expression_reference(DistributedID source, + ReferenceMutator *mutator = NULL, unsigned count = 1); + virtual bool remove_base_expression_reference(ReferenceSource source, + unsigned count = 1); + virtual bool remove_nested_expression_reference(DistributedID source, + unsigned count = 1); + virtual void add_tree_expression_reference(DistributedID source, + unsigned count = 1); + virtual bool remove_tree_expression_reference(DistributedID source, + unsigned count = 1); public: virtual bool remove_operation(RegionTreeForest *forest) = 0; virtual IndexSpaceNode* create_node(IndexSpace handle, @@ -1963,13 +1977,20 @@ namespace Legion { public: virtual bool try_add_canonical_reference(void); virtual bool remove_canonical_reference(void); - virtual void add_expression_reference(std::set &applied_events, - unsigned count = 1); - virtual void add_expression_reference(ReferenceMutator *mutator = NULL, - unsigned count = 1); - virtual bool remove_expression_reference(unsigned count = 1); - virtual void add_expression_tree_reference(unsigned count = 1); - virtual bool remove_expression_tree_reference(unsigned count = 1); + virtual void add_base_expression_reference(ReferenceSource source, + ReferenceMutator *mutator = NULL, unsigned count = 1); + virtual void add_nested_expression_reference(DistributedID source, + std::set &applied_events, unsigned count = 1); + virtual void add_nested_expression_reference(DistributedID source, + ReferenceMutator *mutator = NULL, unsigned count = 1); + virtual bool remove_base_expression_reference(ReferenceSource source, + unsigned count = 1); + virtual bool remove_nested_expression_reference(DistributedID source, + unsigned count = 1); + virtual void add_tree_expression_reference(DistributedID source, + unsigned count = 1); + virtual bool remove_tree_expression_reference(DistributedID source, + unsigned count = 1); public: virtual bool remove_operation(RegionTreeForest *forest); virtual IndexSpaceNode* create_node(IndexSpace handle, diff --git a/runtime/legion/region_tree.inl b/runtime/legion/region_tree.inl index 3e3e3e2838..856622a524 100644 --- a/runtime/legion/region_tree.inl +++ b/runtime/legion/region_tree.inl @@ -1876,7 +1876,7 @@ namespace Legion { #endif // Add the parent and the reference sub->add_parent_operation(this); - sub->add_expression_tree_reference(); + sub->add_tree_expression_reference(this->did); // Then get the realm index space expression ApEvent precondition = sub->get_expr_index_space( &spaces[idx], this->type_tag, false/*need tight result*/); @@ -1946,7 +1946,7 @@ namespace Legion { { // Remove references from our sub expressions for (unsigned idx = 0; idx < sub_expressions.size(); idx++) - if (sub_expressions[idx]->remove_expression_tree_reference()) + if (sub_expressions[idx]->remove_tree_expression_reference(this->did)) delete sub_expressions[idx]; } @@ -1997,7 +1997,7 @@ namespace Legion { forest->remove_union_operation(this, sub_expressions); // Remove our expression reference added by invalidate_operation // and return true if we should be deleted - return this->remove_expression_tree_reference(); + return this->remove_base_resource_ref(IS_EXPR_REF); } //-------------------------------------------------------------------------- @@ -2019,7 +2019,7 @@ namespace Legion { #endif // Add the parent and the reference sub->add_parent_operation(this); - sub->add_expression_tree_reference(); + sub->add_tree_expression_reference(this->did); ApEvent precondition = sub->get_expr_index_space( &spaces[idx], this->type_tag, false/*need tight result*/); if (precondition.exists()) @@ -2089,7 +2089,7 @@ namespace Legion { { // Remove references from our sub expressions for (unsigned idx = 0; idx < sub_expressions.size(); idx++) - if (sub_expressions[idx]->remove_expression_tree_reference()) + if (sub_expressions[idx]->remove_tree_expression_reference(this->did)) delete sub_expressions[idx]; } @@ -2141,7 +2141,7 @@ namespace Legion { forest->remove_intersection_operation(this, sub_expressions); // Remove our expression reference added by invalidate_operation // and return true if we should be deleted - return this->remove_expression_tree_reference(); + return this->remove_base_resource_ref(IS_EXPR_REF); } //-------------------------------------------------------------------------- @@ -2160,7 +2160,7 @@ namespace Legion { { // Special case for when the expressions are the same lhs->add_parent_operation(this); - lhs->add_expression_tree_reference(); + lhs->add_tree_expression_reference(this->did); this->realm_index_space = Realm::IndexSpace::make_empty(); this->tight_index_space = Realm::IndexSpace::make_empty(); this->realm_index_space_ready = ApEvent::NO_AP_EVENT; @@ -2172,8 +2172,8 @@ namespace Legion { // Add the parent and the references lhs->add_parent_operation(this); rhs->add_parent_operation(this); - lhs->add_expression_tree_reference(); - rhs->add_expression_tree_reference(); + lhs->add_tree_expression_reference(this->did); + rhs->add_tree_expression_reference(this->did); ApEvent left_ready = lhs->get_expr_index_space(&lhs_space, this->type_tag, false/*tight*/); ApEvent right_ready = @@ -2239,9 +2239,9 @@ namespace Legion { //-------------------------------------------------------------------------- { if ((rhs != NULL) && (lhs != rhs) && - rhs->remove_expression_tree_reference()) + rhs->remove_tree_expression_reference(this->did)) delete rhs; - if ((lhs != NULL) && lhs->remove_expression_tree_reference()) + if ((lhs != NULL) && lhs->remove_tree_expression_reference(this->did)) delete lhs; } @@ -2294,7 +2294,7 @@ namespace Legion { forest->remove_subtraction_operation(this, lhs, rhs); // Remove our expression reference added by invalidate_operation // and return true if we should be deleted - return this->remove_expression_tree_reference(); + return this->remove_base_resource_ref(IS_EXPR_REF); } ///////////////////////////////////////////////////////////// From a9221c87e9512b1d123dc788fad860e478fc471a Mon Sep 17 00:00:00 2001 From: Mike Bauer Date: Wed, 1 Dec 2021 15:57:18 -0800 Subject: [PATCH 2/9] legion/tools: more fixes for new distributed expressions --- runtime/legion/garbage_collection.h | 4 ++-- runtime/legion/legion_analysis.cc | 6 +++--- runtime/legion/legion_views.cc | 5 ++++- tools/legion_gc.py | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/runtime/legion/garbage_collection.h b/runtime/legion/garbage_collection.h index 837c6be37d..b752d3ede2 100644 --- a/runtime/legion/garbage_collection.h +++ b/runtime/legion/garbage_collection.h @@ -76,7 +76,7 @@ namespace Legion { CONTEXT_REF = 17, RESTRICTED_REF = 18, VERSION_STATE_TREE_REF = 19, - PHYSICAL_MANAGER_REF = 20, + PHYSICAL_USER_REF = 20, LOGICAL_VIEW_REF = 21, REGION_TREE_REF = 22, LAYOUT_DESC_REF = 23, @@ -117,7 +117,7 @@ namespace Legion { "Context Reference", \ "Restricted Reference", \ "Version State Tree Reference", \ - "Physical Manager Reference", \ + "Physical User Reference", \ "Logical View Reference", \ "Region Tree Reference", \ "Layout Description Reference", \ diff --git a/runtime/legion/legion_analysis.cc b/runtime/legion/legion_analysis.cc index 7d2e0953ce..09a6d12949 100644 --- a/runtime/legion/legion_analysis.cc +++ b/runtime/legion/legion_analysis.cc @@ -80,7 +80,7 @@ namespace Legion { #ifdef DEBUG_LEGION assert(expr != NULL); #endif - expr->add_base_expression_reference(IS_EXPR_REF); + expr->add_base_expression_reference(PHYSICAL_USER_REF); } #else //-------------------------------------------------------------------------- @@ -92,7 +92,7 @@ namespace Legion { #ifdef DEBUG_LEGION assert(expr != NULL); #endif - expr->add_base_expression_reference(IS_EXPR_REF); + expr->add_base_expression_reference(PHYSICAL_USER_REF); } #endif @@ -116,7 +116,7 @@ namespace Legion { #ifdef DEBUG_LEGION assert(expr != NULL); #endif - if (expr->remove_base_expression_reference(IS_EXPR_REF)) + if (expr->remove_base_expression_reference(PHYSICAL_USER_REF)) delete expr; } diff --git a/runtime/legion/legion_views.cc b/runtime/legion/legion_views.cc index dcb15c3493..b1750141d7 100644 --- a/runtime/legion/legion_views.cc +++ b/runtime/legion/legion_views.cc @@ -761,7 +761,10 @@ namespace Legion { //-------------------------------------------------------------------------- ExprView::ExprView(const ExprView &rhs) : context(rhs.context), manager(rhs.manager), inst_view(rhs.inst_view), - view_did(0), view_expr(rhs.view_expr), view_volume(rhs.view_volume) + view_expr(rhs.view_expr), view_volume(rhs.view_volume) +#if defined(DEBUG_LEGION_GC) || defined(LEGION_GC) + , view_did(rhs.view_did) +#endif //-------------------------------------------------------------------------- { // should never be called diff --git a/tools/legion_gc.py b/tools/legion_gc.py index 14f7ed4beb..2636660b29 100755 --- a/tools/legion_gc.py +++ b/tools/legion_gc.py @@ -636,7 +636,7 @@ def __init__(self, did, node, handle): self.handle = handle def __repr__(self): - return 'Index Expressions '+str(self.did)+' (Node='+str(self.node)+') ExprID '+str(self.handle) + return 'Index Expression '+str(self.did)+' (Node='+str(self.node)+') ExprID '+str(self.handle) class FieldSpace(Base): def __init__(self, did, node, handle): From bce2b462128b8fa30b63a5394f20ca6a10935cae Mon Sep 17 00:00:00 2001 From: Mike Bauer Date: Wed, 1 Dec 2021 23:48:55 -0800 Subject: [PATCH 3/9] legion: more work on refactoring for distributed expressions to pass provenance information through canonical references --- runtime/legion/garbage_collection.cc | 40 ++++++++++++++++++++++++++++ runtime/legion/garbage_collection.h | 5 ++-- runtime/legion/region_tree.cc | 21 ++++++++------- runtime/legion/region_tree.h | 15 ++++++----- runtime/legion/region_tree.inl | 5 ++-- 5 files changed, 65 insertions(+), 21 deletions(-) diff --git a/runtime/legion/garbage_collection.cc b/runtime/legion/garbage_collection.cc index 9a26d6c64e..f9b909493c 100644 --- a/runtime/legion/garbage_collection.cc +++ b/runtime/legion/garbage_collection.cc @@ -486,6 +486,46 @@ namespace Legion { return true; } + //-------------------------------------------------------------------------- + bool DistributedCollectable::check_resource_and_increment( + DistributedID source, int cnt) + //-------------------------------------------------------------------------- + { + AutoLock gc(gc_lock); + if (current_state == DELETED_STATE) + return false; +#ifdef DEBUG_LEGION + assert(cnt >= 0); +#endif +#ifdef LEGION_GC + log_nested_ref(RESOURCE_REF_KIND, did, local_space, source, cnt); +#endif +#ifndef DEBUG_LEGION_GC + int previous = __sync_fetch_and_add(&resource_references, cnt); +#ifdef DEBUG_LEGION + assert(previous >= 0); +#endif + if (previous == 0) + has_resource_references = true; +#else + resource_references++; + source = LEGION_DISTRIBUTED_ID_FILTER(source); + std::map::iterator finder = + detailed_nested_resource_references.find(source); + if (finder == detailed_nested_resource_references.end()) + detailed_nested_resource_references[source] = cnt; + else + finder->second += cnt; + if (resource_references > cnt) + return true; +#ifdef DEBUG_LEGION + assert(!has_resource_references); +#endif + has_resource_references = true; +#endif + return true; + } + //-------------------------------------------------------------------------- void DistributedCollectable::add_resource_reference(void) //-------------------------------------------------------------------------- diff --git a/runtime/legion/garbage_collection.h b/runtime/legion/garbage_collection.h index b752d3ede2..fd2f8d4a1c 100644 --- a/runtime/legion/garbage_collection.h +++ b/runtime/legion/garbage_collection.h @@ -85,8 +85,7 @@ namespace Legion { TRACE_REF = 26, AGGREGATORE_REF = 27, FIELD_STATE_REF = 28, - CANONICAL_REF = 29, - LAST_SOURCE_REF = 30, + LAST_SOURCE_REF = 29, }; enum ReferenceKind { @@ -126,7 +125,6 @@ namespace Legion { "Physical Trace Reference", \ "Aggregator Reference", \ "Field State Reference", \ - "Canonical Index Space Expression Reference", \ } extern Realm::Logger log_garbage; @@ -336,6 +334,7 @@ namespace Legion { // Atomic check and increment operations inline bool check_valid_and_increment(ReferenceSource source,int cnt = 1); bool check_resource_and_increment(ReferenceSource source ,int cnt = 1); + bool check_resource_and_increment(DistributedID source, int cnt = 1); private: void add_gc_reference(ReferenceMutator *mutator); bool remove_gc_reference(ReferenceMutator *mutator); diff --git a/runtime/legion/region_tree.cc b/runtime/legion/region_tree.cc index 999de75122..a39d423ba3 100644 --- a/runtime/legion/region_tree.cc +++ b/runtime/legion/region_tree.cc @@ -6480,7 +6480,7 @@ namespace Legion { // forest has given us a reference back on it, see if we're the first // ones to write it, if not we can remove the reference now if (!__sync_bool_compare_and_swap(&canonical, NULL, expr)) - expr->remove_canonical_reference(); + expr->remove_canonical_reference(get_distributed_id()); return expr; } @@ -6557,7 +6557,8 @@ namespace Legion { #endif if (canonical == original) forest->remove_canonical_expression(canonical, volume); - else if (canonical->remove_canonical_reference()) + else if (canonical->remove_canonical_reference( + original->get_distributed_id())) delete canonical; } @@ -6636,17 +6637,17 @@ namespace Legion { } //-------------------------------------------------------------------------- - bool IndexSpaceOperation::try_add_canonical_reference(void) + bool IndexSpaceOperation::try_add_canonical_reference(DistributedID source) //-------------------------------------------------------------------------- { - return check_resource_and_increment(CANONICAL_REF); + return check_resource_and_increment(source); } //-------------------------------------------------------------------------- - bool IndexSpaceOperation::remove_canonical_reference(void) + bool IndexSpaceOperation::remove_canonical_reference(DistributedID source) //-------------------------------------------------------------------------- { - return remove_base_resource_ref(CANONICAL_REF); + return remove_nested_resource_ref(source); } //-------------------------------------------------------------------------- @@ -8679,17 +8680,17 @@ namespace Legion { } //-------------------------------------------------------------------------- - bool IndexSpaceNode::try_add_canonical_reference(void) + bool IndexSpaceNode::try_add_canonical_reference(DistributedID source) //-------------------------------------------------------------------------- { - return check_resource_and_increment(CANONICAL_REF); + return check_resource_and_increment(source); } //-------------------------------------------------------------------------- - bool IndexSpaceNode::remove_canonical_reference(void) + bool IndexSpaceNode::remove_canonical_reference(DistributedID source) //-------------------------------------------------------------------------- { - return remove_base_resource_ref(CANONICAL_REF); + return remove_nested_resource_ref(source); } //-------------------------------------------------------------------------- diff --git a/runtime/legion/region_tree.h b/runtime/legion/region_tree.h index 79ee089030..a12b3cde8e 100644 --- a/runtime/legion/region_tree.h +++ b/runtime/legion/region_tree.h @@ -1092,8 +1092,9 @@ namespace Legion { virtual void pack_expression_value(Serializer &rez, AddressSpaceID target) = 0; public: - virtual bool try_add_canonical_reference(void) = 0; - virtual bool remove_canonical_reference(void) = 0; + virtual DistributedID get_distributed_id(void) const = 0; + virtual bool try_add_canonical_reference(DistributedID source) = 0; + virtual bool remove_canonical_reference(DistributedID source) = 0; virtual void add_base_expression_reference(ReferenceSource source, ReferenceMutator *mutator = NULL, unsigned count = 1) = 0; virtual void add_nested_expression_reference(DistributedID source, @@ -1353,8 +1354,9 @@ namespace Legion { virtual void pack_expression_value(Serializer &rez, AddressSpaceID target) = 0; public: - virtual bool try_add_canonical_reference(void); - virtual bool remove_canonical_reference(void); + virtual DistributedID get_distributed_id(void) const { return did; } + virtual bool try_add_canonical_reference(DistributedID source); + virtual bool remove_canonical_reference(DistributedID source); virtual void add_base_expression_reference(ReferenceSource source, ReferenceMutator *mutator = NULL, unsigned count = 1); virtual void add_nested_expression_reference(DistributedID source, @@ -1975,8 +1977,9 @@ namespace Legion { virtual void pack_expression(Serializer &rez, AddressSpaceID target); virtual void pack_expression_value(Serializer &rez,AddressSpaceID target); public: - virtual bool try_add_canonical_reference(void); - virtual bool remove_canonical_reference(void); + virtual DistributedID get_distributed_id(void) const { return did; } + virtual bool try_add_canonical_reference(DistributedID source); + virtual bool remove_canonical_reference(DistributedID source); virtual void add_base_expression_reference(ReferenceSource source, ReferenceMutator *mutator = NULL, unsigned count = 1); virtual void add_nested_expression_reference(DistributedID source, diff --git a/runtime/legion/region_tree.inl b/runtime/legion/region_tree.inl index 856622a524..93a0afe5fd 100644 --- a/runtime/legion/region_tree.inl +++ b/runtime/legion/region_tree.inl @@ -1238,6 +1238,7 @@ namespace Legion { // No need to wait for the event, we know it is already triggered // because we called get_volume on this before we got here get_expr_index_space(&local_space, type_tag, true/*need tight result*/); + const DistributedID local_did = get_distributed_id(); for (std::set::const_iterator it = expressions.begin(); it != expressions.end(); it++) { @@ -1258,7 +1259,7 @@ namespace Legion { // We know that things are the same here // Try to add the expression reference, we can race with deletions // here though so handle the case we're we can't add a reference - if ((*it)->try_add_canonical_reference()) + if ((*it)->try_add_canonical_reference(local_did)) return (*it); else continue; @@ -1380,7 +1381,7 @@ namespace Legion { // If we get here that means we are congruent // Try to add the expression reference, we can race with deletions // here though so handle the case we're we can't add a reference - if ((*it)->try_add_canonical_reference()) + if ((*it)->try_add_canonical_reference(local_did)) return (*it); } // Did not find any congruences so add ourself From 58082fd554dbcdfa91bc8f851e4944d9038cce3b Mon Sep 17 00:00:00 2001 From: Mike Bauer Date: Fri, 10 Dec 2021 01:17:45 -0800 Subject: [PATCH 4/9] legion: fixes for reference counting of index space expressions --- runtime/legion/garbage_collection.cc | 166 +++++ runtime/legion/garbage_collection.h | 54 +- runtime/legion/legion_analysis.cc | 86 ++- runtime/legion/legion_analysis.h | 5 +- runtime/legion/legion_context.h | 14 + runtime/legion/legion_instances.cc | 8 +- runtime/legion/legion_types.h | 103 ++- runtime/legion/region_tree.cc | 1016 ++++++++++++++++++-------- runtime/legion/region_tree.h | 230 ++++-- runtime/legion/region_tree.inl | 131 ++-- runtime/legion/runtime.cc | 89 ++- 11 files changed, 1386 insertions(+), 516 deletions(-) diff --git a/runtime/legion/garbage_collection.cc b/runtime/legion/garbage_collection.cc index f9b909493c..840c685068 100644 --- a/runtime/legion/garbage_collection.cc +++ b/runtime/legion/garbage_collection.cc @@ -447,6 +447,172 @@ namespace Legion { return do_deletion; } + //-------------------------------------------------------------------------- + bool DistributedCollectable::check_valid_and_increment( + ReferenceSource source, int cnt) + //-------------------------------------------------------------------------- + { + AutoLock gc(gc_lock); + if (current_state != VALID_STATE) + return false; +#ifdef DEBUG_LEGION + assert(cnt >= 0); +#endif +#ifdef LEGION_GC + log_base_ref(VALID_REF_KIND, did, local_space, source, cnt); +#endif +#ifndef DEBUG_LEGION_GC + int previous = __sync_fetch_and_add(&valid_references, cnt); +#ifdef DEBUG_LEGION + assert(previous >= 0); +#endif + if (previous == 0) + has_valid_references = true; +#else + valid_references++; + std::map::iterator finder = + detailed_base_valid_references.find(source); + if (finder == detailed_base_valid_references.end()) + detailed_base_valid_references[source] = cnt; + else + finder->second += cnt; + if (valid_references > cnt) + return true; +#ifdef DEBUG_LEGION + assert(!has_valid_references); +#endif + has_valid_references = true; +#endif + return true; + } + + //-------------------------------------------------------------------------- + bool DistributedCollectable::check_valid_and_increment( + DistributedID source, int cnt) + //-------------------------------------------------------------------------- + { + AutoLock gc(gc_lock); + if (current_state != VALID_STATE) + return false; +#ifdef DEBUG_LEGION + assert(cnt >= 0); +#endif +#ifdef LEGION_GC + log_nested_ref(VALID_REF_KIND, did, local_space, source, cnt); +#endif +#ifndef DEBUG_LEGION_GC + int previous = __sync_fetch_and_add(&valid_references, cnt); +#ifdef DEBUG_LEGION + assert(previous >= 0); +#endif + if (previous == 0) + has_valid_references = true; +#else + valid_references++; + source = LEGION_DISTRIBUTED_ID_FILTER(source); + std::map::iterator finder = + detailed_nested_valid_references.find(source); + if (finder == detailed_nested_valid_references.end()) + detailed_nested_valid_references[source] = cnt; + else + finder->second += cnt; + if (valid_references > cnt) + return true; +#ifdef DEBUG_LEGION + assert(!has_valid_references); +#endif + has_valid_references = true; +#endif + return true; + } + + //-------------------------------------------------------------------------- + bool DistributedCollectable::check_gc_and_increment( + ReferenceSource source, int cnt) + //-------------------------------------------------------------------------- + { + AutoLock gc(gc_lock); + if ((current_state == INACTIVE_STATE) || + (current_state == DELETED_STATE) || + (current_state == PENDING_ACTIVE_STATE) || + (current_state == PENDING_INACTIVE_STATE) || + (current_state == PENDING_INACTIVE_INVALID_STATE)) + return false; +#ifdef DEBUG_LEGION + assert(cnt >= 0); +#endif +#ifdef LEGION_GC + log_base_ref(GC_REF_KIND, did, local_space, source, cnt); +#endif +#ifndef DEBUG_LEGION_GC + int previous = __sync_fetch_and_add(&gc_references, cnt); +#ifdef DEBUG_LEGION + assert(previous >= 0); +#endif + if (previous == 0) + has_gc_references = true; +#else + gc_references++; + std::map::iterator finder = + detailed_base_gc_references.find(source); + if (finder == detailed_base_gc_references.end()) + detailed_base_gc_references[source] = cnt; + else + finder->second += cnt; + if (gc_references > cnt) + return true; +#ifdef DEBUG_LEGION + assert(!has_gc_references); +#endif + has_gc_references = true; +#endif + return true; + } + + //-------------------------------------------------------------------------- + bool DistributedCollectable::check_gc_and_increment( + DistributedID source, int cnt) + //-------------------------------------------------------------------------- + { + AutoLock gc(gc_lock); + if ((current_state == INACTIVE_STATE) || + (current_state == DELETED_STATE) || + (current_state == PENDING_ACTIVE_STATE) || + (current_state == PENDING_INACTIVE_STATE) || + (current_state == PENDING_INACTIVE_INVALID_STATE)) + return false; +#ifdef DEBUG_LEGION + assert(cnt >= 0); +#endif +#ifdef LEGION_GC + log_nested_ref(GC_REF_KIND, did, local_space, source, cnt); +#endif +#ifndef DEBUG_LEGION_GC + int previous = __sync_fetch_and_add(&gc_references, cnt); +#ifdef DEBUG_LEGION + assert(previous >= 0); +#endif + if (previous == 0) + has_gc_references = true; +#else + gc_references++; + source = LEGION_DISTRIBUTED_ID_FILTER(source); + std::map::iterator finder = + detailed_nested_gc_references.find(source); + if (finder == detailed_nested_gc_references.end()) + detailed_nested_gc_references[source] = cnt; + else + finder->second += cnt; + if (gc_references > cnt) + return true; +#ifdef DEBUG_LEGION + assert(!has_gc_references); +#endif + has_gc_references = true; +#endif + return true; + } + //-------------------------------------------------------------------------- bool DistributedCollectable::check_resource_and_increment( ReferenceSource source, int cnt) diff --git a/runtime/legion/garbage_collection.h b/runtime/legion/garbage_collection.h index fd2f8d4a1c..7166e010ec 100644 --- a/runtime/legion/garbage_collection.h +++ b/runtime/legion/garbage_collection.h @@ -66,7 +66,7 @@ namespace Legion { REMOTE_DID_REF = 7, PENDING_COLLECTIVE_REF = 8, MEMORY_MANAGER_REF = 9, - COMPOSITE_NODE_REF = 10, + INSTANCE_BUILDER_REF = 10, FIELD_ALLOCATOR_REF = 11, REMOTE_CREATE_REF = 12, INSTANCE_MAPPER_REF = 13, @@ -75,15 +75,15 @@ namespace Legion { NEVER_GC_REF = 16, CONTEXT_REF = 17, RESTRICTED_REF = 18, - VERSION_STATE_TREE_REF = 19, + META_TASK_REF = 19, PHYSICAL_USER_REF = 20, LOGICAL_VIEW_REF = 21, REGION_TREE_REF = 22, LAYOUT_DESC_REF = 23, RUNTIME_REF = 24, - IS_EXPR_REF = 25, + LIVE_EXPR_REF = 25, TRACE_REF = 26, - AGGREGATORE_REF = 27, + AGGREGATOR_REF = 27, FIELD_STATE_REF = 28, LAST_SOURCE_REF = 29, }; @@ -106,7 +106,7 @@ namespace Legion { "Remote Distributed ID Reference", \ "Pending Collective Reference", \ "Memory Manager Reference", \ - "Composite Node Reference", \ + "Instance Builder Reference", \ "Field Allocator Reference", \ "Remote Creation Reference", \ "Instance Mapper Reference", \ @@ -115,13 +115,13 @@ namespace Legion { "Never GC Reference", \ "Context Reference", \ "Restricted Reference", \ - "Version State Tree Reference", \ + "Meta-Task Reference", \ "Physical User Reference", \ "Logical View Reference", \ "Region Tree Reference", \ "Layout Description Reference", \ "Runtime Reference", \ - "Index Space Expression Reference", \ + "Live Index Space Expression Reference", \ "Physical Trace Reference", \ "Aggregator Reference", \ "Field State Reference", \ @@ -331,8 +331,14 @@ namespace Legion { inline bool remove_base_resource_ref(ReferenceSource source, int cnt = 1); inline bool remove_nested_resource_ref(DistributedID source, int cnt = 1); public: +#ifdef DEBUG_LEGION + bool check_valid(void) const { return (current_state == VALID_STATE); } +#endif // Atomic check and increment operations - inline bool check_valid_and_increment(ReferenceSource source,int cnt = 1); + bool check_valid_and_increment(ReferenceSource source,int cnt = 1); + bool check_valid_and_increment(DistributedID source, int cnt = 1); + bool check_gc_and_increment(ReferenceSource source, int cnt = 1); + bool check_gc_and_increment(DistributedID source, int cnt = 1); bool check_resource_and_increment(ReferenceSource source ,int cnt = 1); bool check_resource_and_increment(DistributedID source, int cnt = 1); private: @@ -863,38 +869,6 @@ namespace Legion { #endif } - //-------------------------------------------------------------------------- - inline bool DistributedCollectable::check_valid_and_increment( - ReferenceSource source, int cnt /*=1*/) - //-------------------------------------------------------------------------- - { -#ifdef DEBUG_LEGION - assert(cnt >= 0); -#endif - // Don't support this if we are debugging GC -#ifndef DEBUG_LEGION_GC - // Read the value in an unsafe way at first - int current_cnt = valid_references; - // Don't even both trying if the count is zero - while (current_cnt > 0) - { - const int next_cnt = current_cnt + cnt; - const int prev_cnt = - __sync_val_compare_and_swap(&valid_references, current_cnt, next_cnt); - if (prev_cnt == current_cnt) - { -#ifdef LEGION_GC - log_base_ref(VALID_REF_KIND, did, local_space, source, cnt); -#endif - return true; - } - // Update the current count - current_cnt = prev_cnt; - } -#endif - return false; - } - }; // namespace Internal }; // namespace Legion diff --git a/runtime/legion/legion_analysis.cc b/runtime/legion/legion_analysis.cc index 09a6d12949..90d8440167 100644 --- a/runtime/legion/legion_analysis.cc +++ b/runtime/legion/legion_analysis.cc @@ -3973,8 +3973,41 @@ namespace Legion { // Remove references from any views that we have for (std::set::const_iterator it = all_views.begin(); it != all_views.end(); it++) - if ((*it)->remove_base_valid_ref(AGGREGATORE_REF)) + if ((*it)->remove_base_valid_ref(AGGREGATOR_REF)) delete (*it); + all_views.clear(); + // Remove source precondition expression references + for (std::map::iterator vit = + src_pre.begin(); vit != src_pre.end(); vit++) + { + for (EventFieldExprs::iterator eit = + vit->second.begin(); eit != vit->second.end(); eit++) + { + for (FieldMaskSet::iterator it = + eit->second.begin(); it != eit->second.end(); it++) + if (it->first->remove_base_expression_reference(AGGREGATOR_REF)) + delete it->first; + eit->second.clear(); + } + vit->second.clear(); + } + src_pre.clear(); + // Remove destination precondition expression references + for (std::map::iterator vit = + dst_pre.begin(); vit != dst_pre.end(); vit++) + { + for (EventFieldExprs::iterator eit = + vit->second.begin(); eit != vit->second.end(); eit++) + { + for (FieldMaskSet::iterator it = + eit->second.begin(); it != eit->second.end(); it++) + if (it->first->remove_base_expression_reference(AGGREGATOR_REF)) + delete it->first; + eit->second.clear(); + } + vit->second.clear(); + } + dst_pre.clear(); // Delete all our copy updates for (LegionMap >::aligned:: const_iterator mit = sources.begin(); mit != sources.end(); mit++) @@ -4014,6 +4047,23 @@ namespace Legion { return *this; } + //-------------------------------------------------------------------------- + CopyFillAggregator::Update::Update(IndexSpaceExpression *exp, + const FieldMask &mask, CopyAcrossHelper *helper) + : expr(exp), src_mask(mask), across_helper(helper) + //-------------------------------------------------------------------------- + { + expr->add_base_expression_reference(AGGREGATOR_REF); + } + + //-------------------------------------------------------------------------- + CopyFillAggregator::Update::~Update(void) + //-------------------------------------------------------------------------- + { + if (expr->remove_base_expression_reference(AGGREGATOR_REF)) + delete expr; + } + //-------------------------------------------------------------------------- void CopyFillAggregator::CopyUpdate::record_source_expressions( InstanceFieldExprs &src_exprs) const @@ -4458,6 +4508,7 @@ namespace Legion { #ifdef DEBUG_LEGION assert(!preconditions.empty()); #endif + WrapperReferenceMutator mutator(effects); AutoLock p_lock(pre_lock); EventFieldExprs &pre = reading ? src_pre[view] : dst_pre[view]; for (EventFieldExprs::iterator eit = preconditions.begin(); @@ -4473,13 +4524,23 @@ namespace Legion { FieldMaskSet::iterator finder = event_finder->second.find(it->first); if (finder == event_finder->second.end()) + { + // Keep a reference in case we are deferred + it->first->add_base_expression_reference(AGGREGATOR_REF,&mutator); event_finder->second.insert(it->first, it->second); + } else finder.merge(it->second); } } else // We can just swap this over + { + // Keep references in case we are deferred + for (FieldMaskSet::const_iterator it = + eit->second.begin(); it != eit->second.end(); it++) + it->first->add_base_expression_reference(AGGREGATOR_REF, &mutator); pre[eit->first].swap(eit->second); + } } } @@ -4496,7 +4557,12 @@ namespace Legion { FieldMaskSet::iterator finder = event_pre.find(expr); if (finder == event_pre.end()) + { event_pre.insert(expr, mask); + // Keep a reference in case we are deferred + WrapperReferenceMutator mutator(effects); + expr->add_base_expression_reference(AGGREGATOR_REF, &mutator); + } else finder.merge(mask); } @@ -4608,7 +4674,7 @@ namespace Legion { std::pair::iterator,bool> result = all_views.insert(new_view); if (result.second) - new_view->add_base_valid_ref(AGGREGATORE_REF, this); + new_view->add_base_valid_ref(AGGREGATOR_REF, this); } //-------------------------------------------------------------------------- @@ -9685,6 +9751,10 @@ namespace Legion { std::map *copy_exprs = new std::map(); copy_exprs->swap(to_traverse_exprs); + WrapperReferenceMutator mutator(done_events); + for (std::map::const_iterator + it = copy_exprs->begin(); it != copy_exprs->end(); it++) + it->second->add_base_expression_reference(META_TASK_REF, &mutator); const RtUserEvent done = Runtime::create_rt_user_event(); DeferRayTraceFinishArgs args(target, source, copy_traverse, copy_exprs, expr->get_volume(), handle, done); @@ -13865,7 +13935,7 @@ namespace Legion { //-------------------------------------------------------------------------- { if (local) - expr->add_base_expression_reference(IS_EXPR_REF); + expr->add_base_expression_reference(META_TASK_REF); } //-------------------------------------------------------------------------- @@ -13886,7 +13956,7 @@ namespace Legion { delete dargs->ray_mask; // Remove our expression reference too if (dargs->is_local && - dargs->expr->remove_base_expression_reference(IS_EXPR_REF)) + dargs->expr->remove_base_expression_reference(META_TASK_REF)) delete dargs->expr; } @@ -13918,6 +13988,10 @@ namespace Legion { Runtime::trigger_event(dargs->done, Runtime::merge_events(done_events)); else Runtime::trigger_event(dargs->done); + for (std::map::const_iterator it = + dargs->exprs->begin(); it != dargs->exprs->end(); it++) + if (it->second->remove_base_expression_reference(META_TASK_REF)) + delete it->second; delete dargs->to_traverse; delete dargs->exprs; } @@ -13983,7 +14057,7 @@ namespace Legion { //-------------------------------------------------------------------------- { if (is_local) - expr->add_base_expression_reference(IS_EXPR_REF); + expr->add_base_expression_reference(META_TASK_REF); } //-------------------------------------------------------------------------- @@ -14074,7 +14148,7 @@ namespace Legion { set->register_with_runtime(NULL/*no remote registration needed*/); // Remove our expression reference too if (dargs->is_local && - dargs->expr->remove_base_expression_reference(IS_EXPR_REF)) + dargs->expr->remove_base_expression_reference(META_TASK_REF)) delete dargs->expr; } diff --git a/runtime/legion/legion_analysis.h b/runtime/legion/legion_analysis.h index 01681648bc..58c2205b1e 100644 --- a/runtime/legion/legion_analysis.h +++ b/runtime/legion/legion_analysis.h @@ -1233,9 +1233,8 @@ namespace Legion { class Update { public: Update(IndexSpaceExpression *exp, const FieldMask &mask, - CopyAcrossHelper *helper) - : expr(exp), src_mask(mask), across_helper(helper) { } - virtual ~Update(void) { } + CopyAcrossHelper *helper); + virtual ~Update(void); public: virtual void record_source_expressions( InstanceFieldExprs &src_exprs) const = 0; diff --git a/runtime/legion/legion_context.h b/runtime/legion/legion_context.h index 3f8103aafa..ee2d2b16cb 100644 --- a/runtime/legion/legion_context.h +++ b/runtime/legion/legion_context.h @@ -2063,6 +2063,9 @@ namespace Legion { inline void TaskContext::begin_runtime_call(void) //-------------------------------------------------------------------------- { +#ifdef DEBUG_LEGION + assert(implicit_live_expressions == NULL); +#endif if (overhead_tracker == NULL) return; const long long current = Realm::Clock::current_time_in_nanoseconds(); @@ -2075,6 +2078,17 @@ namespace Legion { inline void TaskContext::end_runtime_call(void) //-------------------------------------------------------------------------- { + if (implicit_live_expressions != NULL) + { + // Remove references to any live index space expressions we have + for (std::vector::const_iterator it = + implicit_live_expressions->begin(); it != + implicit_live_expressions->end(); it++) + if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) + delete (*it); + delete implicit_live_expressions; + implicit_live_expressions = NULL; + } if (overhead_tracker == NULL) return; const long long current = Realm::Clock::current_time_in_nanoseconds(); diff --git a/runtime/legion/legion_instances.cc b/runtime/legion/legion_instances.cc index cbf204d642..f6ccf78777 100644 --- a/runtime/legion/legion_instances.cc +++ b/runtime/legion/legion_instances.cc @@ -1592,7 +1592,7 @@ namespace Legion { //-------------------------------------------------------------------------- { if (local_is) - local_expr->add_base_expression_reference(IS_EXPR_REF); + local_expr->add_base_expression_reference(META_TASK_REF); } //-------------------------------------------------------------------------- @@ -1614,7 +1614,7 @@ namespace Legion { dargs->use_event, dargs->redop, dargs->shadow_instance); // Remove the local expression reference if necessary if (dargs->local_is && - dargs->local_expr->remove_base_expression_reference(IS_EXPR_REF)) + dargs->local_expr->remove_base_expression_reference(META_TASK_REF)) delete dargs->local_expr; } @@ -3022,7 +3022,7 @@ namespace Legion { //-------------------------------------------------------------------------- { if (local_is) - local_expr->add_base_expression_reference(IS_EXPR_REF); + local_expr->add_base_expression_reference(META_TASK_REF); } //-------------------------------------------------------------------------- @@ -3046,7 +3046,7 @@ namespace Legion { dargs->use_event, dargs->redop); // Remove the local expression reference if necessary if (dargs->local_is && - dargs->local_expr->remove_base_expression_reference(IS_EXPR_REF)) + dargs->local_expr->remove_base_expression_reference(META_TASK_REF)) delete dargs->local_expr; } diff --git a/runtime/legion/legion_types.h b/runtime/legion/legion_types.h index ab60882c1f..d152d4f767 100644 --- a/runtime/legion/legion_types.h +++ b/runtime/legion/legion_types.h @@ -1480,43 +1480,7 @@ namespace Legion { class InnerContext;; class TopLevelContext; class RemoteContext; - class LeafContext; - - // Nasty global variable for TLS support of figuring out - // our context implicitly - extern __thread TaskContext *implicit_context; - // Same thing for the runtime - extern __thread Runtime *implicit_runtime; - // Another nasty global variable for tracking the fast - // reservations that we are holding - extern __thread AutoLock *local_lock_list; - // One more nasty global variable that we use for tracking - // the provenance of meta-task operations for profiling - // purposes, this has no bearing on correctness - extern __thread ::legion_unique_id_t implicit_provenance; - // Use this to track if we're inside of a registration - // callback function which we know to be deduplicated - enum RegistrationCallbackMode { - NO_REGISTRATION_CALLBACK = 0, - LOCAL_REGISTRATION_CALLBACK = 1, - GLOBAL_REGISTRATION_CALLBACK = 2, - }; - extern __thread unsigned inside_registration_callback; - - /** - * \class LgTaskArgs - * The base class for all Legion Task arguments - */ - template - struct LgTaskArgs { - public: - LgTaskArgs(::legion_unique_id_t uid) - : provenance(uid), lg_task_id(T::TASK_ID) { } - public: - // In this order for alignment reasons - const ::legion_unique_id_t provenance; - const LgTaskID lg_task_id; - }; + class LeafContext; // legion_trace.h class LegionTrace; @@ -1554,6 +1518,7 @@ namespace Legion { class RegionTreeForest; class CopyIndirection; class IndexSpaceExpression; + class IndexSpaceExprRef; class IndexSpaceOperation; template class IndexSpaceOperationT; template class IndexSpaceUnion; @@ -1641,6 +1606,48 @@ namespace Legion { typedef Mapping::MapperEvent MapperEvent; typedef Mapping::ProfilingMeasurementID ProfilingMeasurementID; + // Nasty global variable for TLS support of figuring out + // our context implicitly + extern __thread TaskContext *implicit_context; + // Same thing for the runtime + extern __thread Runtime *implicit_runtime; + // Another nasty global variable for tracking the fast + // reservations that we are holding + extern __thread AutoLock *local_lock_list; + // One more nasty global variable that we use for tracking + // the provenance of meta-task operations for profiling + // purposes, this has no bearing on correctness + extern __thread ::legion_unique_id_t implicit_provenance; + // Use this to track if we're inside of a registration + // callback function which we know to be deduplicated + enum RegistrationCallbackMode { + NO_REGISTRATION_CALLBACK = 0, + LOCAL_REGISTRATION_CALLBACK = 1, + GLOBAL_REGISTRATION_CALLBACK = 2, + }; + extern __thread unsigned inside_registration_callback; + // This data structure tracks references to any live + // temporary index space expressions that have been + // handed back by the region tree inside the execution + // of a meta-task or a runtime API call + extern __thread + std::vector *implicit_live_expressions; + + /** + * \class LgTaskArgs + * The base class for all Legion Task arguments + */ + template + struct LgTaskArgs { + public: + LgTaskArgs(::legion_unique_id_t uid) + : provenance(uid), lg_task_id(T::TASK_ID) { } + public: + // In this order for alignment reasons + const ::legion_unique_id_t provenance; + const LgTaskID lg_task_id; + }; + #define FRIEND_ALL_RUNTIME_CLASSES \ friend class Legion::Runtime; \ friend class Internal::Runtime; \ @@ -2377,6 +2384,12 @@ namespace Legion { UniqueID local_provenance = Internal::implicit_provenance; // Save whether we are in a registration callback unsigned local_callback = Internal::inside_registration_callback; + // Save any local live expressions that we have + std::vector *local_live_expressions = + Internal::implicit_live_expressions; +#ifdef DEBUG_LEGION + Internal::implicit_live_expressions = NULL; +#endif // Check to see if we have any local locks to notify if (Internal::local_lock_list != NULL) { @@ -2415,6 +2428,11 @@ namespace Legion { Internal::implicit_provenance = local_provenance; // Write the registration callback information back Internal::inside_registration_callback = local_callback; +#ifdef DEBUG_LEGION + assert(Internal::implicit_live_expressions == NULL); +#endif + // Write the local live expressions back + Internal::implicit_live_expressions = local_live_expressions; } //-------------------------------------------------------------------------- @@ -2427,6 +2445,12 @@ namespace Legion { UniqueID local_provenance = Internal::implicit_provenance; // Save whether we are in a registration callback unsigned local_callback = Internal::inside_registration_callback; + // Save any local live expressions that we have + std::vector *local_live_expressions = + Internal::implicit_live_expressions; +#ifdef DEBUG_LEGION + Internal::implicit_live_expressions = NULL; +#endif // Check to see if we have any local locks to notify if (Internal::local_lock_list != NULL) { @@ -2465,6 +2489,11 @@ namespace Legion { Internal::implicit_provenance = local_provenance; // Write the registration callback information back Internal::inside_registration_callback = local_callback; +#ifdef DEBUG_LEGION + assert(Internal::implicit_live_expressions == NULL); +#endif + // Write the local live expressions back + Internal::implicit_live_expressions = local_live_expressions; } #ifdef LEGION_SPY diff --git a/runtime/legion/region_tree.cc b/runtime/legion/region_tree.cc index 7663bd3ee1..249beb85c8 100644 --- a/runtime/legion/region_tree.cc +++ b/runtime/legion/region_tree.cc @@ -2308,7 +2308,8 @@ namespace Legion { return result; } else - return intersect->issue_copy(trace_info, dst_fields, src_fields, + return intersect->issue_copy(trace_info, + dst_fields, src_fields, #ifdef LEGION_SPY src_req.region.get_tree_id(), dst_req.region.get_tree_id(), @@ -2616,7 +2617,7 @@ namespace Legion { RegionNode *src_node = get_node(src_req.region); RegionNode *idx_node = get_node(idx_req.region); RegionNode *dst_node = get_node(dst_req.region); - IndexSpaceExpression *copy_expr = + IndexSpaceExpression *copy_expr = (idx_node->row_source == src_node->row_source) ? idx_node->row_source : intersect_index_spaces(src_node->row_source, idx_node->row_source); // Easy out if we're not going to move anything @@ -2796,8 +2797,8 @@ namespace Legion { RegionNode *src_idx_node = get_node(src_idx_req.region); RegionNode *dst_node = get_node(dst_req.region); RegionNode *dst_idx_node = get_node(dst_idx_req.region); - IndexSpaceExpression *copy_expr = - (src_idx_node->row_source == dst_idx_node->row_source) ? + IndexSpaceExpression *copy_expr = + (src_idx_node->row_source == dst_idx_node->row_source) ? src_idx_node->row_source : intersect_index_spaces( src_idx_node->row_source, dst_idx_node->row_source); // Quick out if there is nothing we're going to copy @@ -5596,43 +5597,66 @@ namespace Legion { //-------------------------------------------------------------------------- IndexSpaceExpression* RegionTreeForest::union_index_spaces( - IndexSpaceExpression *lhs, IndexSpaceExpression *rhs) + IndexSpaceExpression *lhs, IndexSpaceExpression *rhs, + ReferenceMutator *mutator) //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION assert(lhs->type_tag == rhs->type_tag); + assert(lhs->is_valid()); + assert(rhs->is_valid()); #endif if (lhs == rhs) return lhs; if (lhs->is_empty()) return rhs; - lhs = lhs->get_canonical_expression(this); if (rhs->is_empty()) return lhs; - rhs = rhs->get_canonical_expression(this); - if (lhs == rhs) + IndexSpaceExpression *lhs_canon = lhs->get_canonical_expression(this); + IndexSpaceExpression *rhs_canon = rhs->get_canonical_expression(this); + if (lhs_canon == rhs_canon) return lhs; std::vector exprs(2); - if (compare_expressions(lhs, rhs)) + if (compare_expressions(lhs_canon, rhs_canon)) { - exprs[0] = lhs; - exprs[1] = rhs; + exprs[0] = lhs_canon; + exprs[1] = rhs_canon; } else { - exprs[0] = rhs; - exprs[1] = lhs; + exprs[0] = rhs_canon; + exprs[1] = lhs_canon; } - return union_index_spaces(exprs); + IndexSpaceExpression *result = union_index_spaces(exprs); + // Add the live reference + if (mutator == NULL) + { + LocalReferenceMutator local_mutator; + result->add_base_expression_reference(LIVE_EXPR_REF, &local_mutator); + } + else + result->add_base_expression_reference(LIVE_EXPR_REF, mutator); + // Save it in the implicit live expression references + if (implicit_live_expressions == NULL) + implicit_live_expressions = new std::vector(); + implicit_live_expressions->emplace_back(result); + // Remove the gc reference that comes back from finding it in the tree + if (result->remove_live_reference(REGION_TREE_REF)) + assert(false); // should never hit this + return result; } //-------------------------------------------------------------------------- IndexSpaceExpression* RegionTreeForest::union_index_spaces( - const std::set &exprs) + const std::set &exprs, + ReferenceMutator *mutator) //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION assert(!exprs.empty()); + for (std::set::const_iterator it = + exprs.begin(); it != exprs.end(); it++) + assert((*it)->is_valid()); #endif if (exprs.size() == 1) return *(exprs.begin()); @@ -5647,8 +5671,23 @@ namespace Legion { } if (expressions.empty()) return *(exprs.begin()); + LocalReferenceMutator local_mutator; if (expressions.size() == 1) - return expressions[0]; + { + IndexSpaceExpression *result = expressions.back(); + if (exprs.find(result) == exprs.end()) + { + if (mutator == NULL) + result->add_base_expression_reference(LIVE_EXPR_REF,&local_mutator); + else + result->add_base_expression_reference(LIVE_EXPR_REF, mutator); + if (implicit_live_expressions == NULL) + implicit_live_expressions = + new std::vector; + implicit_live_expressions->emplace_back(result); + } + return result; + } // sort them in order by their IDs std::sort(expressions.begin(), expressions.end(), compare_expressions); // remove duplicates @@ -5661,8 +5700,24 @@ namespace Legion { assert(!expressions.empty()); #endif if (expressions.size() == 1) + { + IndexSpaceExpression *result = expressions.back(); + if (exprs.find(result) == exprs.end()) + { + if (mutator == NULL) + result->add_base_expression_reference(LIVE_EXPR_REF, + &local_mutator); + else + result->add_base_expression_reference(LIVE_EXPR_REF, mutator); + if (implicit_live_expressions == NULL) + implicit_live_expressions = + new std::vector; + implicit_live_expressions->emplace_back(result); + } return expressions.back(); + } } + bool first_pass = true; // this helps make sure we don't overflow our stack while (expressions.size() > MAX_EXPRESSION_FANOUT) { @@ -5681,34 +5736,110 @@ namespace Legion { if (expressions.empty()) break; } - next_expressions.push_back(union_index_spaces(temp_expressions)); + IndexSpaceExpression *expr = union_index_spaces(temp_expressions); + if (mutator == NULL) + expr->add_base_expression_reference(REGION_TREE_REF, + &local_mutator); + else + expr->add_base_expression_reference(REGION_TREE_REF, mutator); + // Remove the gc ref that comes back from the union call + if (expr->remove_live_reference(REGION_TREE_REF)) + assert(false); // should never hit this + next_expressions.push_back(expr); } else { - next_expressions.push_back(expressions.back()); + IndexSpaceExpression *expr = expressions.back(); expressions.pop_back(); + if (mutator == NULL) + expr->add_base_expression_reference(REGION_TREE_REF, + &local_mutator); + else + expr->add_base_expression_reference(REGION_TREE_REF, mutator); + next_expressions.push_back(expr); } } + if (!first_pass) + { + // Remove the expression references on the previous set + for (std::vector::const_iterator it = + expressions.begin(); it != expressions.end(); it++) + if ((*it)->remove_base_expression_reference(REGION_TREE_REF)) + delete (*it); + } + else + first_pass = false; expressions.swap(next_expressions); // canonicalize and uniquify them all again + std::set unique_expressions; for (unsigned idx = 0; idx < expressions.size(); idx++) { - IndexSpaceExpression *&expr = expressions[idx]; - expr = expr->get_canonical_expression(this); + IndexSpaceExpression *expr = expressions[idx]; + IndexSpaceExpression *unique = expr->get_canonical_expression(this); + if (unique_expressions.insert(unique).second) + { + if (mutator == NULL) + unique->add_base_expression_reference(REGION_TREE_REF, + &local_mutator); + else + unique->add_base_expression_reference(REGION_TREE_REF, mutator); + } } - std::sort(expressions.begin(), expressions.end(), compare_expressions); - last = std::unique(expressions.begin(), expressions.end()); - if (last != expressions.end()) + // Remove the expression references + for (std::vector::const_iterator it = + expressions.begin(); it != expressions.end(); it++) + if ((*it)->remove_base_expression_reference(REGION_TREE_REF)) + delete (*it); + if (unique_expressions.size() == 1) { - expressions.erase(last, expressions.end()); -#ifdef DEBUG_LEGION - assert(!expressions.empty()); -#endif - if (expressions.size() == 1) - return expressions.back(); + IndexSpaceExpression *result = *(unique_expressions.begin()); + if (exprs.find(result) == exprs.end()) + { + if (mutator == NULL) + result->add_base_expression_reference(LIVE_EXPR_REF, + &local_mutator); + else + result->add_base_expression_reference(LIVE_EXPR_REF, mutator); + if (implicit_live_expressions == NULL) + implicit_live_expressions = + new std::vector(); + implicit_live_expressions->emplace_back(result); + } + // Remove the extra expression reference we added + if (result->remove_base_expression_reference(REGION_TREE_REF)) + assert(false); // should never hit this + return result; } + expressions.resize(unique_expressions.size()); + unsigned index = 0; + for (std::set::const_iterator + it = unique_expressions.begin(); + it != unique_expressions.end(); it++) + expressions[index++] = *it; } - return union_index_spaces(expressions); + IndexSpaceExpression *result = union_index_spaces(expressions); + if (exprs.find(result) == exprs.end()) + { + if (mutator == NULL) + result->add_base_expression_reference(LIVE_EXPR_REF,&local_mutator); + else + result->add_base_expression_reference(LIVE_EXPR_REF, mutator); + if (implicit_live_expressions == NULL) + implicit_live_expressions = new std::vector(); + implicit_live_expressions->emplace_back(result); + } + // Remove the reference added by the trie traversal + if (result->remove_live_reference(REGION_TREE_REF)) + assert(false); // should never hit this deletion + if (!first_pass) + { + // Remove the extra references on the expression vector we added + for (std::vector::const_iterator it = + expressions.begin(); it != expressions.end(); it++) + if ((*it)->remove_base_expression_reference(REGION_TREE_REF)) + delete (*it); + } + return result; } //-------------------------------------------------------------------------- @@ -5732,7 +5863,8 @@ namespace Legion { { IndexSpaceExpression *result = NULL; ExpressionTrieNode *next = NULL; - if (finder->second->find_operation(expressions, result, next)) + if (finder->second->find_operation(expressions, result, next) && + result->try_add_live_reference(REGION_TREE_REF)) return result; if (creator == NULL) { @@ -5789,7 +5921,8 @@ namespace Legion { //-------------------------------------------------------------------------- IndexSpaceExpression* RegionTreeForest::intersect_index_spaces( - IndexSpaceExpression *lhs, IndexSpaceExpression *rhs) + IndexSpaceExpression *lhs, IndexSpaceExpression *rhs, + ReferenceMutator *mutator) //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION @@ -5799,33 +5932,53 @@ namespace Legion { return lhs; if (lhs->is_empty()) return lhs; - lhs = lhs->get_canonical_expression(this); if (rhs->is_empty()) return rhs; - rhs = rhs->get_canonical_expression(this); - if (lhs == rhs) + IndexSpaceExpression *lhs_canon = lhs->get_canonical_expression(this); + IndexSpaceExpression *rhs_canon = rhs->get_canonical_expression(this); + if (lhs_canon == rhs_canon) return lhs; std::vector exprs(2); - if (compare_expressions(lhs, rhs)) + if (compare_expressions(lhs_canon, rhs_canon)) { - exprs[0] = lhs; - exprs[1] = rhs; + exprs[0] = lhs_canon; + exprs[1] = rhs_canon; } else { - exprs[0] = rhs; - exprs[1] = lhs; + exprs[0] = rhs_canon; + exprs[1] = lhs_canon; } - return intersect_index_spaces(exprs); + IndexSpaceExpression *result = intersect_index_spaces(exprs); + // Add the live reference + if (mutator == NULL) + { + LocalReferenceMutator local_mutator; + result->add_base_expression_reference(LIVE_EXPR_REF, &local_mutator); + } + else + result->add_base_expression_reference(LIVE_EXPR_REF, mutator); + // Save it in the implicit live expression references + if (implicit_live_expressions == NULL) + implicit_live_expressions = new std::vector(); + implicit_live_expressions->emplace_back(result); + // Remove the gc reference that comes back with the trie traversal + if (result->remove_live_reference(REGION_TREE_REF)) + assert(false); // should never hit this + return result; } //-------------------------------------------------------------------------- IndexSpaceExpression* RegionTreeForest::intersect_index_spaces( - const std::set &exprs) + const std::set &exprs, + ReferenceMutator *mutator) //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION assert(!exprs.empty()); + for (std::set::const_iterator it = + exprs.begin(); it != exprs.end(); it++) + assert((*it)->is_valid()); #endif if (exprs.size() == 1) return *(exprs.begin()); @@ -5844,6 +5997,7 @@ namespace Legion { // remove duplicates std::vector::iterator last = std::unique(expressions.begin(), expressions.end()); + LocalReferenceMutator local_mutator; if (last != expressions.end()) { expressions.erase(last, expressions.end()); @@ -5851,8 +6005,24 @@ namespace Legion { assert(!expressions.empty()); #endif if (expressions.size() == 1) - return expressions.back(); + { + IndexSpaceExpression *result = expressions.back(); + if (exprs.find(result) == exprs.end()) + { + if (mutator == NULL) + result->add_base_expression_reference(LIVE_EXPR_REF, + &local_mutator); + else + result->add_base_expression_reference(LIVE_EXPR_REF, mutator); + if (implicit_live_expressions == NULL) + implicit_live_expressions = + new std::vector; + implicit_live_expressions->emplace_back(result); + } + return result; + } } + bool first_pass = true; // this helps make sure we don't overflow our stack while (expressions.size() > MAX_EXPRESSION_FANOUT) { @@ -5871,37 +6041,138 @@ namespace Legion { if (expressions.empty()) break; } - next_expressions.push_back( - intersect_index_spaces(temp_expressions)); + IndexSpaceExpression *expr = + intersect_index_spaces(temp_expressions); + if (mutator == NULL) + expr->add_base_expression_reference(REGION_TREE_REF, + &local_mutator); + else + expr->add_base_expression_reference(REGION_TREE_REF, mutator); + // Remove the gc ref that comes back from the union call + if (expr->remove_live_reference(REGION_TREE_REF)) + assert(false); // should never hit this + next_expressions.push_back(expr); } else { - next_expressions.push_back(expressions.back()); + IndexSpaceExpression *expr = expressions.back(); expressions.pop_back(); + if (mutator == NULL) + expr->add_base_expression_reference(REGION_TREE_REF, + &local_mutator); + else + expr->add_base_expression_reference(REGION_TREE_REF, mutator); + next_expressions.push_back(expr); } } + if (!first_pass) + { + // Remove the expression references on the previous set + for (std::vector::const_iterator it = + expressions.begin(); it != expressions.end(); it++) + if ((*it)->remove_base_expression_reference(REGION_TREE_REF)) + delete (*it); + } + else + first_pass = false; expressions.swap(next_expressions); // canonicalize and uniquify them all again + std::set unique_expressions; for (unsigned idx = 0; idx < expressions.size(); idx++) { - IndexSpaceExpression *&expr = expressions[idx]; - if (expr->is_empty()) - return expr; - expr = expr->get_canonical_expression(this); + IndexSpaceExpression *expr = expressions[idx]; + IndexSpaceExpression *unique = expr->get_canonical_expression(this); + if (unique->is_empty()) + { + // Add a reference to the unique expression + if (exprs.find(unique) == exprs.end()) + { + if (mutator == NULL) + unique->add_base_expression_reference(LIVE_EXPR_REF, + &local_mutator); + else + unique->add_base_expression_reference(LIVE_EXPR_REF, mutator); + if (implicit_live_expressions == NULL) + implicit_live_expressions = + new std::vector; + implicit_live_expressions->emplace_back(unique); + } + // Remove references on all the things we no longer need + for (std::set:: + const_iterator it = unique_expressions.begin(); it != + unique_expressions.end(); it++) + if ((*it)->remove_base_expression_reference(REGION_TREE_REF)) + delete (*it); + for (std::vector::const_iterator it = + expressions.begin(); it != expressions.end(); it++) + if ((*it)->remove_base_expression_reference(REGION_TREE_REF)) + delete (*it); + return unique; + } + if (unique_expressions.insert(unique).second) + { + if (mutator == NULL) + unique->add_base_expression_reference(REGION_TREE_REF, + &local_mutator); + else + unique->add_base_expression_reference(REGION_TREE_REF, mutator); + } } - std::sort(expressions.begin(), expressions.end(), compare_expressions); - last = std::unique(expressions.begin(), expressions.end()); - if (last != expressions.end()) + // Remove the expression references + for (std::vector::const_iterator it = + expressions.begin(); it != expressions.end(); it++) + if ((*it)->remove_base_expression_reference(REGION_TREE_REF)) + delete (*it); + if (unique_expressions.size() == 1) { - expressions.erase(last, expressions.end()); -#ifdef DEBUG_LEGION - assert(!expressions.empty()); -#endif - if (expressions.size() == 1) - return expressions.back(); + IndexSpaceExpression *result = *(unique_expressions.begin()); + if (exprs.find(result) == exprs.end()) + { + if (mutator == NULL) + result->add_base_expression_reference(LIVE_EXPR_REF, + &local_mutator); + else + result->add_base_expression_reference(LIVE_EXPR_REF, mutator); + if (implicit_live_expressions == NULL) + implicit_live_expressions = + new std::vector(); + implicit_live_expressions->emplace_back(result); + } + // Remove the extra expression reference we added + if (result->remove_base_expression_reference(REGION_TREE_REF)) + assert(false); // should never hit this + return result; } + expressions.resize(unique_expressions.size()); + unsigned index = 0; + for (std::set::const_iterator + it = unique_expressions.begin(); + it != unique_expressions.end(); it++) + expressions[index++] = *it; + } + IndexSpaceExpression *result = intersect_index_spaces(expressions); + if (exprs.find(result) == exprs.end()) + { + if (mutator == NULL) + result->add_base_expression_reference(LIVE_EXPR_REF,&local_mutator); + else + result->add_base_expression_reference(LIVE_EXPR_REF, mutator); + if (implicit_live_expressions == NULL) + implicit_live_expressions = new std::vector; + implicit_live_expressions->emplace_back(result); + } + // Remove the reference added by the trie traversal + if (result->remove_live_reference(REGION_TREE_REF)) + assert(false); // should never hit this deletion + if (!first_pass) + { + // Remove the extra references on the expression vector we added + for (std::vector::const_iterator it = + expressions.begin(); it != expressions.end(); it++) + if ((*it)->remove_base_expression_reference(REGION_TREE_REF)) + delete (*it); } - return intersect_index_spaces(expressions); + return result; } //-------------------------------------------------------------------------- @@ -5925,7 +6196,8 @@ namespace Legion { { IndexSpaceExpression *result = NULL; ExpressionTrieNode *next = NULL; - if (finder->second->find_operation(expressions, result, next)) + if (finder->second->find_operation(expressions, result, next) && + result->try_add_live_reference(REGION_TREE_REF)) return result; if (creator == NULL) { @@ -5986,11 +6258,14 @@ namespace Legion { //-------------------------------------------------------------------------- IndexSpaceExpression* RegionTreeForest::subtract_index_spaces( IndexSpaceExpression *lhs, IndexSpaceExpression *rhs, - OperationCreator *creator/*=NULL*/) + OperationCreator *creator/*=NULL*/, + ReferenceMutator *mutator/*=NULL*/) //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION assert(lhs->type_tag == rhs->type_tag); + assert(lhs->is_valid()); + assert(rhs->is_valid()); #endif // Handle a few easy cases if (creator == NULL) @@ -6005,71 +6280,93 @@ namespace Legion { expressions[1] = rhs->get_canonical_expression(this); const IndexSpaceExprID key = expressions[0]->expr_id; // See if we can find it in read-only mode + IndexSpaceExpression *result = NULL; { AutoLock l_lock(lookup_is_op_lock,1,false/*exclusive*/); std::map::const_iterator finder = difference_ops.find(key); if (finder != difference_ops.end()) { - IndexSpaceExpression *result = NULL; + IndexSpaceExpression *expr = NULL; ExpressionTrieNode *next = NULL; - if (finder->second->find_operation(expressions, result, next)) - return result; - if (creator == NULL) + if (finder->second->find_operation(expressions, expr, next) && + expr->try_add_live_reference(REGION_TREE_REF)) + result = expr; + if (result == NULL) { - DifferenceOpCreator diff_creator(this, lhs->type_tag, - expressions[0], expressions[1]); - return next->find_or_create_operation(expressions, diff_creator); + if (creator == NULL) + { + DifferenceOpCreator diff_creator(this, lhs->type_tag, + expressions[0], expressions[1]); + result = next->find_or_create_operation(expressions,diff_creator); + } + else + result = next->find_or_create_operation(expressions, *creator); } - else - return next->find_or_create_operation(expressions, *creator); } } - ExpressionTrieNode *node = NULL; - if (creator == NULL) + if (result == NULL) { - DifferenceOpCreator diff_creator(this, lhs->type_tag, - expressions[0], expressions[1]); - // Didn't find it, retake the lock, see if we lost the race - // and if not make the actual trie node - AutoLock l_lock(lookup_is_op_lock); - // See if we lost the race - std::map::const_iterator - finder = difference_ops.find(key); - if (finder == difference_ops.end()) + ExpressionTrieNode *node = NULL; + if (creator == NULL) { - // Didn't lose the race so make the node - node = new ExpressionTrieNode(0/*depth*/, expressions[0]->expr_id); - difference_ops[key] = node; - } - else - node = finder->second; + DifferenceOpCreator diff_creator(this, lhs->type_tag, + expressions[0], expressions[1]); + // Didn't find it, retake the lock, see if we lost the race + // and if not make the actual trie node + AutoLock l_lock(lookup_is_op_lock); + // See if we lost the race + std::map::const_iterator + finder = difference_ops.find(key); + if (finder == difference_ops.end()) + { + // Didn't lose the race so make the node + node = new ExpressionTrieNode(0/*depth*/, expressions[0]->expr_id); + difference_ops[key] = node; + } + else + node = finder->second; #ifdef DEBUG_LEGION - assert(node != NULL); + assert(node != NULL); #endif - return node->find_or_create_operation(expressions, diff_creator); - } - else - { - // Didn't find it, retake the lock, see if we lost the race - // and if not make the actual trie node - AutoLock l_lock(lookup_is_op_lock); - // See if we lost the race - std::map::const_iterator - finder = difference_ops.find(key); - if (finder == difference_ops.end()) - { - // Didn't lose the race so make the node - node = new ExpressionTrieNode(0/*depth*/, expressions[0]->expr_id); - difference_ops[key] = node; + result = node->find_or_create_operation(expressions, diff_creator); } else - node = finder->second; + { + // Didn't find it, retake the lock, see if we lost the race + // and if not make the actual trie node + AutoLock l_lock(lookup_is_op_lock); + // See if we lost the race + std::map::const_iterator + finder = difference_ops.find(key); + if (finder == difference_ops.end()) + { + // Didn't lose the race so make the node + node = new ExpressionTrieNode(0/*depth*/, expressions[0]->expr_id); + difference_ops[key] = node; + } + else + node = finder->second; #ifdef DEBUG_LEGION - assert(node != NULL); + assert(node != NULL); #endif - return node->find_or_create_operation(expressions, *creator); + result = node->find_or_create_operation(expressions, *creator); + } } + if (mutator == NULL) + { + LocalReferenceMutator local_mutator; + result->add_base_expression_reference(LIVE_EXPR_REF, &local_mutator); + } + else + result->add_base_expression_reference(LIVE_EXPR_REF, mutator); + if (implicit_live_expressions == NULL) + implicit_live_expressions = new std::vector; + implicit_live_expressions->emplace_back(result); + // Remove the gc reference that comes back from finding it in the tree + if (result->remove_live_reference(REGION_TREE_REF)) + assert(false); // should never hit this + return result; } //-------------------------------------------------------------------------- @@ -6108,37 +6405,27 @@ namespace Legion { } //-------------------------------------------------------------------------- - void RegionTreeForest::invalidate_index_space_expression( - const std::vector &parents) + void RegionTreeForest::invalidate_index_space_operations( + const std::vector &derived) //-------------------------------------------------------------------------- { // Two phases here: in read-only made figure out the set of operations // we are going to invalidate but don't remove them yet - std::deque to_remove; - { - AutoLock l_lock(lookup_is_op_lock,1,false/*exclusive*/); - for (std::vector::const_iterator it = - parents.begin(); it != parents.end(); it++) - (*it)->invalidate_operation(to_remove); - } - if (to_remove.empty()) - return; - // Now retake the lock and do the removal - std::deque to_delete; + std::vector invalidated; + invalidated.reserve(derived.size()); { AutoLock l_lock(lookup_is_op_lock); - for (std::deque::const_iterator it = - to_remove.begin(); it != to_remove.end(); it++) + for (std::vector::const_iterator it = + derived.begin(); it != derived.end(); it++) { - if ((*it)->remove_operation(this)) - to_delete.push_back(*it); + if ((*it)->invalidate_operation()) + invalidated.push_back(*it); } } - if (to_delete.empty()) - return; - for (std::deque::const_iterator it = - to_delete.begin(); it != to_delete.end(); it++) - delete (*it); + for (std::vector::const_iterator it = + invalidated.begin(); it != invalidated.end(); it++) + if ((*it)->remove_base_gc_ref(REGION_TREE_REF)) + delete (*it); } //-------------------------------------------------------------------------- @@ -6375,8 +6662,14 @@ namespace Legion { result->send_remote_gc_decrement(source); return result; } - RemoteExpressionCreator creator(this, derez); - return creator.consume(); + TypeTag type_tag; + derez.deserialize(type_tag); + RemoteExpressionCreator creator(this, type_tag, derez); + NT_TemplateHelper::demux(type_tag, &creator); +#ifdef DEBUG_LEGION + assert(creator.operation != NULL); +#endif + return creator.operation; } ///////////////////////////////////////////////////////////// @@ -6415,7 +6708,7 @@ namespace Legion { //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION - assert(parent_operations.empty()); + assert(derived_operations.empty()); #endif } @@ -6426,7 +6719,7 @@ namespace Legion { { const TightenIndexSpaceArgs *targs = (const TightenIndexSpaceArgs*)args; targs->proxy_this->tighten_index_space(); - if (targs->proxy_this->remove_base_expression_reference(IS_EXPR_REF)) + if (targs->proxy_dc->remove_base_resource_ref(META_TASK_REF)) delete targs->proxy_this; } @@ -6439,25 +6732,56 @@ namespace Legion { } //-------------------------------------------------------------------------- - void IndexSpaceExpression::add_parent_operation(IndexSpaceOperation *op) + void IndexSpaceExpression::add_derived_operation(IndexSpaceOperation *op) //-------------------------------------------------------------------------- { AutoLock e_lock(expr_lock); #ifdef DEBUG_LEGION - assert(parent_operations.find(op) == parent_operations.end()); + assert(derived_operations.find(op) == derived_operations.end()); #endif - parent_operations.insert(op); + derived_operations.insert(op); } //-------------------------------------------------------------------------- - void IndexSpaceExpression::remove_parent_operation(IndexSpaceOperation *op) + void IndexSpaceExpression::remove_derived_operation(IndexSpaceOperation *op) //-------------------------------------------------------------------------- { AutoLock e_lock(expr_lock); #ifdef DEBUG_LEGION - assert(parent_operations.find(op) != parent_operations.end()); + assert(derived_operations.find(op) != derived_operations.end()); #endif - parent_operations.erase(op); + derived_operations.erase(op); + } + + //-------------------------------------------------------------------------- + void IndexSpaceExpression::invalidate_derived_operations(DistributedID did, + RegionTreeForest *context) + //-------------------------------------------------------------------------- + { + // Traverse upwards for any derived operations and invalidate them + std::vector derived; + { + AutoLock e_lock(expr_lock,1,false/*exclusive*/); + if (!derived_operations.empty()) + { + derived.reserve(derived_operations.size()); + for (std::set::const_iterator it = + derived_operations.begin(); it != derived_operations.end(); it++) + { + (*it)->add_tree_expression_reference(did); + derived.push_back(*it); + } + } + } + if (!derived.empty()) + { + context->invalidate_index_space_operations(derived); + // Remove any references that we have on the parents + for (std::vector::const_iterator it = + derived.begin(); it != derived.end(); it++) + if ((*it)->remove_tree_expression_reference(did)) + delete (*it); + } } //-------------------------------------------------------------------------- @@ -6479,8 +6803,11 @@ namespace Legion { // If the canonical expression is not ourself, then the region tree // forest has given us a reference back on it, see if we're the first // ones to write it, if not we can remove the reference now + const DistributedID did = get_distributed_id(); if (!__sync_bool_compare_and_swap(&canonical, NULL, expr)) - expr->remove_canonical_reference(get_distributed_id()); + expr->remove_canonical_reference(did); + else // We're the first so add our resource reference + expr->add_tree_expression_reference(did); return expr; } @@ -6545,23 +6872,6 @@ namespace Legion { origin, &wait_for); } - //-------------------------------------------------------------------------- - /*static*/ void IndexSpaceExpression::finalize_canonical(size_t volume, - RegionTreeForest *forest, - IndexSpaceExpression *original, - IndexSpaceExpression *canonical) - //-------------------------------------------------------------------------- - { -#ifdef DEBUG_LEGION - assert(canonical != NULL); -#endif - if (canonical == original) - forest->remove_canonical_expression(canonical, volume); - else if (canonical->remove_canonical_reference( - original->get_distributed_id())) - delete canonical; - } - ///////////////////////////////////////////////////////////// // Index Space Operation ///////////////////////////////////////////////////////////// @@ -6574,10 +6884,11 @@ namespace Legion { ctx->runtime->get_available_distributed_id(), ctx->runtime->address_space), context(ctx), origin_expr(this), op_kind(kind), invalidated(0) +#ifdef DEBUG_LEGION + , tree_active(true) +#endif //-------------------------------------------------------------------------- { - // We always keep a reference on ourself until we get invalidated - add_base_resource_ref(IS_EXPR_REF); #ifdef LEGION_GC log_garbage.info("GC Index Expr %lld %d %lld", LEGION_DISTRIBUTED_ID_FILTER(this->did), local_space, expr_id); @@ -6592,11 +6903,17 @@ namespace Legion { DistributedCollectable(ctx->runtime, did, owner), context(ctx), origin_expr(origin), op_kind(REMOTE_EXPRESSION_KIND), invalidated(0) +#ifdef DEBUG_LEGION + , tree_active(true) +#endif //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION assert(!is_owner()); #endif + // Keep a gc reference to ensure that this remain active until it + // is made invalid on the owner node + add_base_gc_ref(REMOTE_DID_REF); #ifdef LEGION_GC log_garbage.info("GC Index Expr %lld %d %lld", LEGION_DISTRIBUTED_ID_FILTER(this->did), local_space, expr_id); @@ -6607,47 +6924,121 @@ namespace Legion { IndexSpaceOperation::~IndexSpaceOperation(void) //-------------------------------------------------------------------------- { - if (canonical != NULL) - { + } + + //-------------------------------------------------------------------------- + void IndexSpaceOperation::notify_active(ReferenceMutator *mutator) + //-------------------------------------------------------------------------- + { #ifdef DEBUG_LEGION - assert(has_volume); + assert(tree_active); // should only happen once #endif - IndexSpaceExpression::finalize_canonical(volume,context,this,canonical); - } + } + + //-------------------------------------------------------------------------- + void IndexSpaceOperation::notify_valid(ReferenceMutator *mutator) + //-------------------------------------------------------------------------- + { + // If we're not the owner send a valid reference to the owner if (!is_owner()) - context->unregister_remote_expression(expr_id); + send_remote_valid_increment(owner_space, mutator); + if ((canonical != NULL) && (canonical != this) && + !canonical->try_add_canonical_reference(did)) + { + // We were unsuccessful at adding our canonical reference so + // remove the resource reference to the canonical object and + // and mark that we no longer have a canonical expression + if (canonical->remove_tree_expression_reference(did)) + delete canonical; + canonical = NULL; + } } //-------------------------------------------------------------------------- - void IndexSpaceOperation::notify_active(ReferenceMutator *mutator) + void IndexSpaceOperation::notify_invalid(ReferenceMutator *mutator) //-------------------------------------------------------------------------- { - // If we're not the owner send a gc reference to the owner if (!is_owner()) - send_remote_gc_increment(owner_space, mutator); + send_remote_valid_decrement(owner_space, mutator); + // If we have a canonical reference that is not ourselves then + // we need to remove the nested reference that we are holding on it too + if ((canonical != NULL) && (canonical != this) && + canonical->remove_canonical_reference(did)) + // should never actually delete it since we have a resource ref too + assert(false); + } + + //-------------------------------------------------------------------------- + void IndexSpaceOperation::InactiveFunctor::apply(AddressSpaceID target) + //-------------------------------------------------------------------------- + { + op->send_remote_gc_decrement(target, mutator); } //-------------------------------------------------------------------------- void IndexSpaceOperation::notify_inactive(ReferenceMutator *mutator) //-------------------------------------------------------------------------- { - // Remove the remote gc reference on the owner - if (!is_owner()) - send_remote_gc_decrement(owner_space, mutator); +#ifdef DEBUG_LEGION + // Should only go through one cycle of active to not active + assert(tree_active); + tree_active = false; +#endif + if (is_owner()) + { + // Send the removal of the gc references on any remote nodes + if (has_remote_instances()) + { + InactiveFunctor functor(this, mutator); + map_over_remote_instances(functor); + } + } + else + context->unregister_remote_expression(expr_id); + // Invalidate any derived operations + invalidate_derived_operations(did, context); + // Remove this operation from the region tree + remove_operation(); + if (canonical != NULL) + { + if (canonical == this) + { +#ifdef DEBUG_LEGION + assert(has_volume); +#endif + context->remove_canonical_expression(this, volume); + } + else if (canonical->remove_tree_expression_reference(did)) + delete canonical; + } } //-------------------------------------------------------------------------- bool IndexSpaceOperation::try_add_canonical_reference(DistributedID source) //-------------------------------------------------------------------------- { - return check_resource_and_increment(source); + return check_gc_and_increment(source); } //-------------------------------------------------------------------------- bool IndexSpaceOperation::remove_canonical_reference(DistributedID source) //-------------------------------------------------------------------------- { - return remove_nested_resource_ref(source); + return remove_nested_gc_ref(source); + } + + //-------------------------------------------------------------------------- + bool IndexSpaceOperation::try_add_live_reference(ReferenceSource source) + //-------------------------------------------------------------------------- + { + return check_gc_and_increment(source); + } + + //-------------------------------------------------------------------------- + bool IndexSpaceOperation::remove_live_reference(ReferenceSource source) + //-------------------------------------------------------------------------- + { + return remove_base_gc_ref(source); } //-------------------------------------------------------------------------- @@ -6658,10 +7049,10 @@ namespace Legion { if (mutator == NULL) { LocalReferenceMutator local_mutator; - add_base_gc_ref(source, &local_mutator, count); + add_base_valid_ref(source, &local_mutator, count); } else - add_base_gc_ref(source, mutator, count); + add_base_valid_ref(source, mutator, count); } //-------------------------------------------------------------------------- @@ -6681,10 +7072,10 @@ namespace Legion { if (mutator == NULL) { LocalReferenceMutator local_mutator; - add_nested_gc_ref(source, &local_mutator, count); + add_nested_valid_ref(source, &local_mutator, count); } else - add_nested_gc_ref(source, mutator, count); + add_nested_valid_ref(source, mutator, count); } //-------------------------------------------------------------------------- @@ -6692,7 +7083,7 @@ namespace Legion { ReferenceSource source, unsigned count) //-------------------------------------------------------------------------- { - return remove_base_gc_ref(source, NULL/*mutator*/, count); + return remove_base_valid_ref(source, NULL/*mutator*/, count); } //-------------------------------------------------------------------------- @@ -6700,7 +7091,7 @@ namespace Legion { DistributedID source, unsigned count) //-------------------------------------------------------------------------- { - return remove_nested_gc_ref(source, NULL/*mutator*/, count); + return remove_nested_valid_ref(source, NULL/*mutator*/, count); } //-------------------------------------------------------------------------- @@ -6719,57 +7110,13 @@ namespace Legion { return remove_nested_resource_ref(id, count); } - //-------------------------------------------------------------------------- - void IndexSpaceOperation::invalidate_operation( - std::deque &to_remove) - //-------------------------------------------------------------------------- - { - // See if we're the first one here, there can be a race with - // multiple invalidations occurring at the same time - if (__sync_fetch_and_add(&invalidated, 1) > 0) - return; - // Add ourselves to the list if we're here first - to_remove.push_back(this); - // The expression that we added in the constructor flows back in - // the 'to_remove' data structure - std::vector parents; - { - // Have to get a read-only copy of these while holding the lock - AutoLock i_lock(inter_lock,1,false/*exclusive*/); - // If we don't have any parent operations then we're done - if (parent_operations.empty()) - return; - parents.resize(parent_operations.size()); - unsigned idx = 0; - for (std::set::const_iterator it = - parent_operations.begin(); it != - parent_operations.end(); it++, idx++) - { - // Add a reference to prevent the parents from being collected - // as we're traversing up the tree - (*it)->add_tree_expression_reference(did); - parents[idx] = (*it); - } - } - // Now continue up the tree with the parents which we are temporarily - // holding a reference to in order to prevent a collection race - for (std::vector::const_iterator it = - parents.begin(); it != parents.end(); it++) - { - (*it)->invalidate_operation(to_remove); - // Remove the reference when we're done with the parents - if ((*it)->remove_tree_expression_reference(did)) - delete (*it); - } - } - ///////////////////////////////////////////////////////////// // Operation Creator ///////////////////////////////////////////////////////////// //-------------------------------------------------------------------------- - OperationCreator::OperationCreator(void) - : result(NULL) + OperationCreator::OperationCreator(RegionTreeForest *f) + : forest(f), result(NULL) //-------------------------------------------------------------------------- { } @@ -6779,11 +7126,8 @@ namespace Legion { //-------------------------------------------------------------------------- { // If we still have a result then it's because it wasn't consumed need - // we need to remove it's reference that was added by the - // IndexSpaceOperation constructor - // We know the operation was never added to the region tree so we - // can pass in a NULL pointer to the region tree forest - if ((result != NULL) && result->remove_operation(NULL/*forest*/)) + // we need to remove it's reference that was added by the constructor + if ((result != NULL) && result->remove_base_resource_ref(REGION_TREE_REF)) delete result; } @@ -6806,9 +7150,11 @@ namespace Legion { #ifdef DEBUG_LEGION assert(result != NULL); #endif - IndexSpaceExpression *temp = result; - result = NULL; - return temp; + // Add an expression reference here since this is going to be put + // into the region tree expression trie data structure, the reference + // will be removed when the expressions is removed from the trie + result->add_base_gc_ref(REGION_TREE_REF); + return result; } ///////////////////////////////////////////////////////////// @@ -6981,13 +7327,17 @@ namespace Legion { { // We're the node that should have the operation // Check to see if we've made the operation yet - if (local_operation != NULL) + if ((local_operation != NULL) && + local_operation->try_add_live_reference(REGION_TREE_REF)) return local_operation; // Operation doesn't exist yet, retake the lock and try to make it AutoLock t_lock(trie_lock); - if (local_operation != NULL) + if ((local_operation != NULL) && + local_operation->try_add_live_reference(REGION_TREE_REF)) return local_operation; local_operation = creator.consume(); + if (!local_operation->try_add_live_reference(REGION_TREE_REF)) + assert(false); // should never hit this return local_operation; } else if (expressions.size() == (depth+2)) @@ -7001,7 +7351,8 @@ namespace Legion { AutoLock t_lock(trie_lock,1,false/*exclusive*/); std::map::const_iterator op_finder = operations.find(target_expr); - if (op_finder != operations.end()) + if ((op_finder != operations.end()) && + op_finder->second->try_add_live_reference(REGION_TREE_REF)) return op_finder->second; std::map::const_iterator node_finder = nodes.find(target_expr); @@ -7015,7 +7366,8 @@ namespace Legion { AutoLock t_lock(trie_lock); std::map::const_iterator op_finder = operations.find(target_expr); - if (op_finder != operations.end()) + if ((op_finder != operations.end()) && + op_finder->second->try_add_live_reference(REGION_TREE_REF)) return op_finder->second; // Still don't have the op std::map::const_iterator @@ -7025,6 +7377,8 @@ namespace Legion { // Didn't find the sub-node, so make the operation here IndexSpaceExpression *result = creator.consume(); operations[target_expr] = result; + if (!result->try_add_live_reference(REGION_TREE_REF)) + assert(false); // should never hit this return result; } else @@ -7383,6 +7737,9 @@ namespace Legion { realm_index_space_set(Runtime::create_rt_user_event()), tight_index_space_set(Runtime::create_rt_user_event()), tight_index_space(false), tree_valid(is_owner()) +#ifdef DEBUG_LEGION + , tree_active(true) +#endif //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION @@ -7417,13 +7774,6 @@ namespace Legion { IndexSpaceNode::~IndexSpaceNode(void) //-------------------------------------------------------------------------- { - if (canonical != NULL) - { -#ifdef DEBUG_LEGION - assert(has_volume); -#endif - IndexSpaceExpression::finalize_canonical(volume,context,this,canonical); - } // Remove ourselves from the context if (registered_with_runtime) context->remove_node(handle); @@ -7444,19 +7794,56 @@ namespace Legion { } //-------------------------------------------------------------------------- - void IndexSpaceNode::notify_valid(ReferenceMutator *mutator) + void IndexSpaceNode::notify_active(ReferenceMutator *mutator) //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION - assert(tree_valid || !is_owner()); + assert(tree_active); #endif + } + + //-------------------------------------------------------------------------- + void IndexSpaceNode::notify_valid(ReferenceMutator *mutator) + //-------------------------------------------------------------------------- + { // If we're not the owner, we add a valid reference to the owner if (!is_owner()) send_remote_valid_increment(owner_space, mutator); + if ((canonical != NULL) && (canonical != this) && + !canonical->try_add_canonical_reference(did)) + { + // We were unsuccessful at adding our canonical reference so + // remove the resource reference to the canonical object and + // and mark that we no longer have a canonical expression + if (canonical->remove_tree_expression_reference(did)) + delete canonical; + canonical = NULL; + } + } + + //-------------------------------------------------------------------------- + void IndexSpaceNode::notify_invalid(ReferenceMutator *mutator) + //-------------------------------------------------------------------------- + { + if (is_owner()) + { + AutoLock n_lock(node_lock); + // First time we become invalid then the tree is no longer valid + // Any later valid states are just for expression references + tree_valid = false; + } + else + send_remote_valid_decrement(owner_space, mutator); + // If we have a canonical reference that is not ourselves then + // we need to remove the nested reference that we are holding on it too + if ((canonical != NULL) && (canonical != this) && + canonical->remove_canonical_reference(did)) + // should never actually delete it since we have a resource ref too + assert(false); } //-------------------------------------------------------------------------- - void IndexSpaceNode::InvalidFunctor::apply(AddressSpaceID target) + void IndexSpaceNode::InactiveFunctor::apply(AddressSpaceID target) //-------------------------------------------------------------------------- { std::map::iterator finder = @@ -7469,57 +7856,38 @@ namespace Legion { } //-------------------------------------------------------------------------- - void IndexSpaceNode::notify_invalid(ReferenceMutator *mutator) + void IndexSpaceNode::notify_inactive(ReferenceMutator *mutator) //-------------------------------------------------------------------------- { - if (is_owner()) - { - AutoLock n_lock(node_lock); #ifdef DEBUG_LEGION - assert(tree_valid); + // Should only become active one time + assert(tree_active); + tree_active = false; #endif - tree_valid = false; - // Send the invalidations - if ((send_references == 0) && has_remote_instances()) + if (is_owner()) + { + // Send the removal of the gc references on any remote nodes + if (has_remote_instances()) { - // Make sure invalidation are not handled before send effects - InvalidFunctor functor(this, mutator, send_effects); + InactiveFunctor functor(this, mutator, send_effects); map_over_remote_instances(functor); } } else - send_remote_valid_decrement(owner_space, mutator); - } - - //-------------------------------------------------------------------------- - void IndexSpaceNode::notify_inactive(ReferenceMutator *mutator) - //-------------------------------------------------------------------------- - { - // Traverse upwards for any parent operations and invalidate them - std::vector parents; + context->unregister_remote_expression(expr_id); + // Invalidate any derived operations + invalidate_derived_operations(did, context); + if (canonical != NULL) { - AutoLock n_lock(node_lock,1,false/*exclusive*/); - if (!parent_operations.empty()) + if (canonical == this) { - parents.resize(parent_operations.size()); - unsigned idx = 0; - for (std::set::const_iterator it = - parent_operations.begin(); it != - parent_operations.end(); it++, idx++) - { - (*it)->add_tree_expression_reference(did); - parents[idx] = (*it); - } +#ifdef DEBUG_LEGION + assert(has_volume); +#endif + context->remove_canonical_expression(this, volume); } - } - if (!parents.empty()) - { - context->invalidate_index_space_expression(parents); - // Remove any references that we have on the parents - for (std::vector::const_iterator it = - parents.begin(); it != parents.end(); it++) - if ((*it)->remove_tree_expression_reference(did)) - delete (*it); + else if (canonical->remove_tree_expression_reference(did)) + delete canonical; } } @@ -8216,13 +8584,6 @@ namespace Legion { // Remove this from the remote instances since we did // not actually end up sending it filter_remote_instances(target); - // Send the invalidations - if (remove_reference && !tree_valid && has_remote_instances()) - { - // Make sure invalidation are not handled before send effects - InvalidFunctor functor(this, &mutator, send_effects); - map_over_remote_instances(functor); - } } if (remove_reference && parent->remove_nested_resource_ref(did)) delete parent; @@ -8285,13 +8646,6 @@ namespace Legion { assert(send_references > 0); #endif remove_reference = (--send_references == 0); - // Send the invalidations - if (remove_reference && !tree_valid && has_remote_instances()) - { - // Make sure invalidation are not handled before send effects - InvalidFunctor functor(this, &mutator, send_effects); - map_over_remote_instances(functor); - } } } if (remove_reference && parent->remove_nested_resource_ref(did)) @@ -8307,14 +8661,6 @@ namespace Legion { { AutoLock n_lock(node_lock); remove_reference = (--send_references == 0); - // Send the invalidations - if (is_owner() && remove_reference && - !tree_valid && has_remote_instances()) - { - // Make sure invalidation are not handled before send effects - InvalidFunctor functor(this, &mutator, send_effects); - map_over_remote_instances(functor); - } } if (remove_reference && parent->remove_nested_resource_ref(did)) delete parent; @@ -8683,14 +9029,28 @@ namespace Legion { bool IndexSpaceNode::try_add_canonical_reference(DistributedID source) //-------------------------------------------------------------------------- { - return check_resource_and_increment(source); + return check_gc_and_increment(source); } //-------------------------------------------------------------------------- bool IndexSpaceNode::remove_canonical_reference(DistributedID source) //-------------------------------------------------------------------------- { - return remove_nested_resource_ref(source); + return remove_nested_gc_ref(source); + } + + //-------------------------------------------------------------------------- + bool IndexSpaceNode::try_add_live_reference(ReferenceSource source) + //-------------------------------------------------------------------------- + { + return check_gc_and_increment(source); + } + + //-------------------------------------------------------------------------- + bool IndexSpaceNode::remove_live_reference(ReferenceSource source) + //-------------------------------------------------------------------------- + { + return remove_base_gc_ref(source); } //-------------------------------------------------------------------------- @@ -8701,10 +9061,10 @@ namespace Legion { if (mutator == NULL) { LocalReferenceMutator local_mutator; - add_base_gc_ref(source, &local_mutator, count); + add_base_valid_ref(source, &local_mutator, count); } else - add_base_gc_ref(source, mutator, count); + add_base_valid_ref(source, mutator, count); } //-------------------------------------------------------------------------- @@ -8724,10 +9084,10 @@ namespace Legion { if (mutator == NULL) { LocalReferenceMutator local_mutator; - add_nested_gc_ref(source, &local_mutator, count); + add_nested_valid_ref(source, &local_mutator, count); } else - add_nested_gc_ref(source, mutator, count); + add_nested_valid_ref(source, mutator, count); } //-------------------------------------------------------------------------- @@ -8735,7 +9095,7 @@ namespace Legion { ReferenceSource source, unsigned count) //-------------------------------------------------------------------------- { - return remove_base_gc_ref(source, NULL/*mutator*/, count); + return remove_base_valid_ref(source, NULL/*mutator*/, count); } //-------------------------------------------------------------------------- @@ -8743,7 +9103,7 @@ namespace Legion { DistributedID source, unsigned count) //-------------------------------------------------------------------------- { - return remove_nested_gc_ref(source, NULL/*mutator*/, count); + return remove_nested_valid_ref(source, NULL/*mutator*/, count); } //-------------------------------------------------------------------------- @@ -8762,13 +9122,6 @@ namespace Legion { return remove_nested_resource_ref(id, count); } - //-------------------------------------------------------------------------- - bool IndexSpaceNode::remove_operation(RegionTreeForest *forest) - //-------------------------------------------------------------------------- - { - return remove_base_resource_ref(IS_EXPR_REF); - } - //-------------------------------------------------------------------------- bool IndexSpaceNode::intersects_with(IndexSpaceNode *rhs, bool compute) //-------------------------------------------------------------------------- @@ -8946,7 +9299,10 @@ namespace Legion { parent->add_nested_resource_ref(did); color_space->add_nested_resource_ref(did); if (has_complete && complete) + { + parent->add_nested_expression_reference(did); union_expr.store(parent); + } else union_expr.store(NULL); #ifdef DEBUG_LEGION @@ -8981,7 +9337,10 @@ namespace Legion { parent->add_nested_resource_ref(did); color_space->add_nested_resource_ref(did); if (has_complete && complete) + { + parent->add_nested_expression_reference(did); union_expr.store(parent); + } else union_expr.store(NULL); #ifdef DEBUG_LEGION @@ -9147,6 +9506,10 @@ namespace Legion { delete (*it); partition_trackers.clear(); } + // Remove the reference on our union expression if we have one + IndexSpaceExpression *expr = union_expr.load(); + if ((expr != NULL) && expr->remove_nested_expression_reference(did)) + delete expr; } //-------------------------------------------------------------------------- @@ -9847,10 +10210,15 @@ namespace Legion { // We can always write the result immediately since we know // that the common sub-expression code will give the same // result if there is a race - union_expr.store(context->union_index_spaces(child_spaces)); + IndexSpaceExpression *expr = context->union_index_spaces(child_spaces); + expr->add_nested_expression_reference(did); + union_expr.store(expr); } else // if we're complete the parent is our expression + { + parent->add_nested_expression_reference(did); union_expr.store(parent); + } return union_expr.load(); } diff --git a/runtime/legion/region_tree.h b/runtime/legion/region_tree.h index a12b3cde8e..ddd6e93b3b 100644 --- a/runtime/legion/region_tree.h +++ b/runtime/legion/region_tree.h @@ -68,13 +68,15 @@ namespace Legion { */ class OperationCreator { public: - OperationCreator(void); + OperationCreator(RegionTreeForest *f); virtual ~OperationCreator(void); public: void produce(IndexSpaceOperation *op); IndexSpaceExpression* consume(void); public: virtual void create_operation(void) = 0; + public: + RegionTreeForest *const forest; protected: IndexSpaceOperation *result; }; @@ -813,10 +815,19 @@ namespace Legion { // if these operations have been requested before and if so will // return the common sub-expression, if not we will actually do // the computation and memoize it for the future + // + // Note that you do not need to worry about reference counting + // expressions returned from these methods inside of tasks because + // we implicitly add references to them and store them in the + // implicit_live_expression data structure and then remove the + // references after the meta-task or runtime call is done executing. + IndexSpaceExpression* union_index_spaces(IndexSpaceExpression *lhs, - IndexSpaceExpression *rhs); + IndexSpaceExpression *rhs, + ReferenceMutator *mutator = NULL); IndexSpaceExpression* union_index_spaces( - const std::set &exprs); + const std::set &exprs, + ReferenceMutator *mutator = NULL); protected: // Internal version IndexSpaceExpression* union_index_spaces( @@ -824,27 +835,36 @@ namespace Legion { OperationCreator *creator = NULL); public: IndexSpaceExpression* intersect_index_spaces( - IndexSpaceExpression *lhs, - IndexSpaceExpression *rhs); + IndexSpaceExpression *lhs, + IndexSpaceExpression *rhs, + ReferenceMutator *mutator = NULL); IndexSpaceExpression* intersect_index_spaces( - const std::set &exprs); + const std::set &exprs, + ReferenceMutator *mutator = NULL); protected: IndexSpaceExpression* intersect_index_spaces( const std::vector &exprs, OperationCreator *creator = NULL); public: IndexSpaceExpression* subtract_index_spaces(IndexSpaceExpression *lhs, - IndexSpaceExpression *rhs, OperationCreator *creator = NULL); + IndexSpaceExpression *rhs, OperationCreator *creator = NULL, + ReferenceMutator *mutator = NULL); public: IndexSpaceExpression* find_canonical_expression(IndexSpaceExpression *ex); void remove_canonical_expression(IndexSpaceExpression *expr, size_t vol); private: static inline bool compare_expressions(IndexSpaceExpression *one, IndexSpaceExpression *two); + struct CompareExpressions { + public: + inline bool operator()(IndexSpaceExpression *one, + IndexSpaceExpression *two) const + { return compare_expressions(one, two); } + }; public: // Methods for removing index space expression when they are done - void invalidate_index_space_expression( - const std::vector &parents); + void invalidate_index_space_operations( + const std::vector &derived); void remove_union_operation(IndexSpaceOperation *expr, const std::vector &exprs); void remove_intersection_operation(IndexSpaceOperation *expr, @@ -1025,12 +1045,14 @@ namespace Legion { static const LgTaskID TASK_ID = LG_TIGHTEN_INDEX_SPACE_TASK_ID; public: - TightenIndexSpaceArgs(IndexSpaceExpression *proxy) + TightenIndexSpaceArgs(IndexSpaceExpression *proxy, + DistributedCollectable *dc) : LgTaskArgs(implicit_provenance), - proxy_this(proxy) - { proxy->add_base_expression_reference(IS_EXPR_REF); } + proxy_this(proxy), proxy_dc(dc) + { proxy_dc->add_base_resource_ref(META_TASK_REF); } public: IndexSpaceExpression *const proxy_this; + DistributedCollectable *const proxy_dc; }; public: template @@ -1092,9 +1114,14 @@ namespace Legion { virtual void pack_expression_value(Serializer &rez, AddressSpaceID target) = 0; public: +#ifdef DEBUG_LEGION + virtual bool is_valid(void) const = 0; +#endif virtual DistributedID get_distributed_id(void) const = 0; virtual bool try_add_canonical_reference(DistributedID source) = 0; virtual bool remove_canonical_reference(DistributedID source) = 0; + virtual bool try_add_live_reference(ReferenceSource source) = 0; + virtual bool remove_live_reference(ReferenceSource source) = 0; virtual void add_base_expression_reference(ReferenceSource source, ReferenceMutator *mutator = NULL, unsigned count = 1) = 0; virtual void add_nested_expression_reference(DistributedID source, @@ -1110,7 +1137,6 @@ namespace Legion { virtual bool remove_tree_expression_reference(DistributedID source, unsigned count = 1) = 0; public: - virtual bool remove_operation(RegionTreeForest *forest) = 0; virtual IndexSpaceNode* create_node(IndexSpace handle, DistributedID did, RtEvent initialized, std::set *applied) = 0; @@ -1177,6 +1203,7 @@ namespace Legion { bool compact,LayoutConstraintKind *unsat_kind = NULL, unsigned *unsat_index = NULL,void **piece_list =NULL, size_t *piece_list_size = NULL) = 0; + // Return the expression with a resource ref on the expression virtual IndexSpaceExpression* create_layout_expression( const void *piece_list, size_t piece_list_size) = 0; virtual bool meets_layout_expression(IndexSpaceExpression *expr, @@ -1188,8 +1215,10 @@ namespace Legion { static void handle_tighten_index_space(const void *args); static AddressSpaceID get_owner_space(IndexSpaceExprID id, Runtime *rt); public: - void add_parent_operation(IndexSpaceOperation *op); - void remove_parent_operation(IndexSpaceOperation *op); + void add_derived_operation(IndexSpaceOperation *op); + void remove_derived_operation(IndexSpaceOperation *op); + void invalidate_derived_operations(DistributedID did, + RegionTreeForest *context); public: inline bool is_empty(void) { @@ -1304,22 +1333,85 @@ namespace Legion { RegionTreeForest *forest, AddressSpaceID source, bool &is_local,bool &is_index_space,IndexSpace &handle, IndexSpaceExprID &remote_expr_id, RtEvent &wait_for); - static void finalize_canonical(size_t volume, RegionTreeForest *forest, - IndexSpaceExpression *original, - IndexSpaceExpression *canonical); public: const TypeTag type_tag; const IndexSpaceExprID expr_id; private: LocalLock &expr_lock; protected: - std::set parent_operations; + std::set derived_operations; IndexSpaceExpression *canonical; size_t volume; bool has_volume; bool empty, has_empty; }; + /** + * This is a move-only object that tracks temporary references to + * index space expressions that are returned from region tree ops + */ + class IndexSpaceExprRef { + public: + IndexSpaceExprRef(void) : expr(NULL) { } + IndexSpaceExprRef(IndexSpaceExpression *e, ReferenceMutator *m = NULL) + : expr(e) + { + if (expr != NULL) + { + if (m == NULL) + { + LocalReferenceMutator local_mutator; + expr->add_base_expression_reference(LIVE_EXPR_REF, &local_mutator); + } + else + expr->add_base_expression_reference(LIVE_EXPR_REF, m); + } + } + IndexSpaceExprRef(const IndexSpaceExprRef &rhs) = delete; + IndexSpaceExprRef(IndexSpaceExprRef &&rhs) + : expr(rhs.expr) + { + rhs.expr = NULL; + } + ~IndexSpaceExprRef(void) + { + if ((expr != NULL) && + expr->remove_base_expression_reference(LIVE_EXPR_REF)) + delete expr; + } + IndexSpaceExprRef& operator=(const IndexSpaceExprRef &rhs) = delete; + inline IndexSpaceExprRef& operator=(IndexSpaceExprRef &&rhs) + { + if ((expr != NULL) && + expr->remove_base_expression_reference(LIVE_EXPR_REF)) + delete expr; + expr = rhs.expr; + rhs.expr = NULL; + return *this; + } + public: + inline bool operator==(const IndexSpaceExprRef &rhs) const + { + if (expr == NULL) + return (rhs.expr == NULL); + if (rhs.expr == NULL) + return false; + return (expr->expr_id == rhs.expr->expr_id); + } + inline bool operator<(const IndexSpaceExprRef &rhs) const + { + if (expr == NULL) + return (rhs.expr != NULL); + if (rhs.expr == NULL) + return false; + return (expr->expr_id < rhs.expr->expr_id); + } + inline IndexSpaceExpression* operator->(void) { return expr; } + inline IndexSpaceExpression* operator&(void) { return expr; } + protected: + IndexSpaceExpression *expr; + }; + class IndexSpaceOperation : public IndexSpaceExpression, public DistributedCollectable { public: @@ -1330,6 +1422,17 @@ namespace Legion { REMOTE_EXPRESSION_KIND, INSTANCE_EXPRESSION_KIND, }; + public: + class InactiveFunctor { + public: + InactiveFunctor(IndexSpaceOperation *o, ReferenceMutator *m) + : op(o), mutator(m) { } + public: + void apply(AddressSpaceID target); + public: + IndexSpaceOperation *const op; + ReferenceMutator *const mutator; + }; public: IndexSpaceOperation(TypeTag tag, OperationKind kind, RegionTreeForest *ctx); @@ -1340,9 +1443,8 @@ namespace Legion { public: virtual void notify_active(ReferenceMutator *mutator); virtual void notify_inactive(ReferenceMutator *mutator); - // We should never be using valid references for index space expressions - virtual void notify_valid(ReferenceMutator *mutator) { assert(false); } - virtual void notify_invalid(ReferenceMutator *mutator) { assert(false); } + virtual void notify_valid(ReferenceMutator *mutator); + virtual void notify_invalid(ReferenceMutator *mutator); public: virtual ApEvent get_expr_index_space(void *result, TypeTag tag, bool need_tight_result) = 0; @@ -1354,9 +1456,14 @@ namespace Legion { virtual void pack_expression_value(Serializer &rez, AddressSpaceID target) = 0; public: +#ifdef DEBUG_LEGION + virtual bool is_valid(void) const { return check_valid(); } +#endif virtual DistributedID get_distributed_id(void) const { return did; } virtual bool try_add_canonical_reference(DistributedID source); virtual bool remove_canonical_reference(DistributedID source); + virtual bool try_add_live_reference(ReferenceSource source); + virtual bool remove_live_reference(ReferenceSource source); virtual void add_base_expression_reference(ReferenceSource source, ReferenceMutator *mutator = NULL, unsigned count = 1); virtual void add_nested_expression_reference(DistributedID source, @@ -1372,20 +1479,22 @@ namespace Legion { virtual bool remove_tree_expression_reference(DistributedID source, unsigned count = 1); public: - virtual bool remove_operation(RegionTreeForest *forest) = 0; + virtual bool invalidate_operation(void) = 0; + virtual void remove_operation(void) = 0; virtual IndexSpaceNode* create_node(IndexSpace handle, DistributedID did, RtEvent initialized, std::set *applied) = 0; - public: - void invalidate_operation(std::deque &to_remove); public: RegionTreeForest *const context; IndexSpaceOperation *const origin_expr; const OperationKind op_kind; protected: mutable LocalLock inter_lock; + std::atomic invalidated; +#ifdef DEBUG_LEGION private: - int invalidated; + bool tree_active; +#endif }; template @@ -1406,7 +1515,8 @@ namespace Legion { virtual void pack_expression(Serializer &rez, AddressSpaceID target); virtual void pack_expression_value(Serializer &rez, AddressSpaceID target) = 0; - virtual bool remove_operation(RegionTreeForest *forest) = 0; + virtual bool invalidate_operation(void) = 0; + virtual void remove_operation(void) = 0; virtual IndexSpaceNode* create_node(IndexSpace handle, DistributedID did, RtEvent initialized, std::set *applied); @@ -1504,7 +1614,8 @@ namespace Legion { IndexSpaceUnion& operator=(const IndexSpaceUnion &rhs); public: virtual void pack_expression_value(Serializer &rez,AddressSpaceID target); - virtual bool remove_operation(RegionTreeForest *forest); + virtual bool invalidate_operation(void); + virtual void remove_operation(void); protected: const std::vector sub_expressions; }; @@ -1513,7 +1624,7 @@ namespace Legion { public: UnionOpCreator(RegionTreeForest *f, TypeTag t, const std::vector &e) - : forest(f), type_tag(t), exprs(e) { } + : OperationCreator(f), type_tag(t), exprs(e) { } public: template static inline void demux(UnionOpCreator *creator) @@ -1525,7 +1636,6 @@ namespace Legion { virtual void create_operation(void) { NT_TemplateHelper::demux(type_tag, this); } public: - RegionTreeForest *const forest; const TypeTag type_tag; const std::vector &exprs; }; @@ -1544,7 +1654,8 @@ namespace Legion { IndexSpaceIntersection& operator=(const IndexSpaceIntersection &rhs); public: virtual void pack_expression_value(Serializer &rez,AddressSpaceID target); - virtual bool remove_operation(RegionTreeForest *forest); + virtual bool invalidate_operation(void); + virtual void remove_operation(void); protected: const std::vector sub_expressions; }; @@ -1553,7 +1664,7 @@ namespace Legion { public: IntersectionOpCreator(RegionTreeForest *f, TypeTag t, const std::vector &e) - : forest(f), type_tag(t), exprs(e) { } + : OperationCreator(f), type_tag(t), exprs(e) { } public: template static inline void demux(IntersectionOpCreator *creator) @@ -1565,7 +1676,6 @@ namespace Legion { virtual void create_operation(void) { NT_TemplateHelper::demux(type_tag, this); } public: - RegionTreeForest *const forest; const TypeTag type_tag; const std::vector &exprs; }; @@ -1584,7 +1694,8 @@ namespace Legion { IndexSpaceDifference& operator=(const IndexSpaceDifference &rhs); public: virtual void pack_expression_value(Serializer &rez,AddressSpaceID target); - virtual bool remove_operation(RegionTreeForest *forest); + virtual bool invalidate_operation(void); + virtual void remove_operation(void); protected: IndexSpaceExpression *const lhs; IndexSpaceExpression *const rhs; @@ -1594,7 +1705,7 @@ namespace Legion { public: DifferenceOpCreator(RegionTreeForest *f, TypeTag t, IndexSpaceExpression *l, IndexSpaceExpression *r) - : forest(f), type_tag(t), lhs(l), rhs(r) { } + : OperationCreator(f), type_tag(t), lhs(l), rhs(r) { } public: template static inline void demux(DifferenceOpCreator *creator) @@ -1606,7 +1717,6 @@ namespace Legion { virtual void create_operation(void) { NT_TemplateHelper::demux(type_tag, this); } public: - RegionTreeForest *const forest; const TypeTag type_tag; IndexSpaceExpression *const lhs; IndexSpaceExpression *const rhs; @@ -1631,7 +1741,8 @@ namespace Legion { InstanceExpression& operator=(const InstanceExpression &rhs); public: virtual void pack_expression_value(Serializer &rez,AddressSpaceID target); - virtual bool remove_operation(RegionTreeForest *forest); + virtual bool invalidate_operation(void); + virtual void remove_operation(void); }; /** @@ -1653,14 +1764,14 @@ namespace Legion { RemoteExpression& operator=(const RemoteExpression &op); public: virtual void pack_expression_value(Serializer &rez,AddressSpaceID target); - virtual bool remove_operation(RegionTreeForest *forest); + virtual bool invalidate_operation(void); + virtual void remove_operation(void); }; - class RemoteExpressionCreator : public OperationCreator { + class RemoteExpressionCreator { public: - RemoteExpressionCreator(RegionTreeForest *f, Deserializer &d) - : forest(f), type_tag(unpack_type_tag(d)), derez(d) - { NT_TemplateHelper::demux(type_tag, this); } + RemoteExpressionCreator(RegionTreeForest *f, TypeTag t, Deserializer &d) + : forest(f), type_tag(t), derez(d), operation(NULL) { } public: template static inline void demux(RemoteExpressionCreator *creator) @@ -1673,24 +1784,18 @@ namespace Legion { creator->derez.deserialize(owner_space); IndexSpaceOperation *origin; creator->derez.deserialize(origin); - creator->produce( +#ifdef DEBUG_LEGION + assert(creator->operation == NULL); +#endif + creator->operation = new RemoteExpression(creator->forest, expr_id, did, - owner_space, origin, creator->type_tag, creator->derez)); - } - public: - // Nothing to do for this - virtual void create_operation(void) { } - public: - static inline TypeTag unpack_type_tag(Deserializer &derez) - { - TypeTag tag; - derez.deserialize(tag); - return tag; + owner_space, origin, creator->type_tag, creator->derez); } public: RegionTreeForest *const forest; const TypeTag type_tag; Deserializer &derez; + IndexSpaceOperation *operation; }; /** @@ -1867,10 +1972,10 @@ namespace Legion { const AddressSpaceID source; Serializer &rez; }; - class InvalidFunctor { + class InactiveFunctor { public: - InvalidFunctor(IndexSpaceNode *n, ReferenceMutator *m, - std::map &effects) + InactiveFunctor(IndexSpaceNode *n, ReferenceMutator *m, + std::map &effects) : node(n), mutator(m), send_effects(effects) { } public: void apply(AddressSpaceID target); @@ -1890,6 +1995,7 @@ namespace Legion { public: IndexSpaceNode& operator=(const IndexSpaceNode &rhs); public: + virtual void notify_active(ReferenceMutator *mutator); virtual void notify_valid(ReferenceMutator *mutator); virtual void notify_invalid(ReferenceMutator *mutator); virtual void notify_inactive(ReferenceMutator *mutator); @@ -1977,9 +2083,14 @@ namespace Legion { virtual void pack_expression(Serializer &rez, AddressSpaceID target); virtual void pack_expression_value(Serializer &rez,AddressSpaceID target); public: +#ifdef DEBUG_LEGION + virtual bool is_valid(void) const { return check_valid(); } +#endif virtual DistributedID get_distributed_id(void) const { return did; } virtual bool try_add_canonical_reference(DistributedID source); virtual bool remove_canonical_reference(DistributedID source); + virtual bool try_add_live_reference(ReferenceSource source); + virtual bool remove_live_reference(ReferenceSource source); virtual void add_base_expression_reference(ReferenceSource source, ReferenceMutator *mutator = NULL, unsigned count = 1); virtual void add_nested_expression_reference(DistributedID source, @@ -1995,7 +2106,6 @@ namespace Legion { virtual bool remove_tree_expression_reference(DistributedID source, unsigned count = 1); public: - virtual bool remove_operation(RegionTreeForest *forest); virtual IndexSpaceNode* create_node(IndexSpace handle, DistributedID did, RtEvent initialized, std::set *applied) = 0; @@ -2140,6 +2250,10 @@ namespace Legion { bool tight_index_space; // Keep track of whether we're still valid on the owner bool tree_valid; +#ifdef DEBUG_LEGION + // Keep track of whether we are active, should only happen once + bool tree_active; +#endif }; /** diff --git a/runtime/legion/region_tree.inl b/runtime/legion/region_tree.inl index 93a0afe5fd..7c5ed0420c 100644 --- a/runtime/legion/region_tree.inl +++ b/runtime/legion/region_tree.inl @@ -1126,11 +1126,11 @@ namespace Legion { { if (rects == NULL) { - if (!space.dense()) + if (space.dense()) + return this; + else // Make a new expression for the bounding box return new InstanceExpression(&space.bounds,1/*size*/,context); - else // if we're dense we can just use ourselves - return this; } else { @@ -1867,6 +1867,8 @@ namespace Legion { sub_expressions(to_union) //-------------------------------------------------------------------------- { + // Add an resource ref that will be removed by the OperationCreator + this->add_base_resource_ref(REGION_TREE_REF); std::set preconditions; std::vector > spaces(sub_expressions.size()); for (unsigned idx = 0; idx < sub_expressions.size(); idx++) @@ -1876,7 +1878,7 @@ namespace Legion { assert(sub->get_canonical_expression(this->context) == sub); #endif // Add the parent and the reference - sub->add_parent_operation(this); + sub->add_derived_operation(this); sub->add_tree_expression_reference(this->did); // Then get the realm index space expression ApEvent precondition = sub->get_expr_index_space( @@ -1900,7 +1902,7 @@ namespace Legion { if (!this->realm_index_space_ready.has_triggered() || !valid_event.has_triggered()) { - IndexSpaceExpression::TightenIndexSpaceArgs args(this); + IndexSpaceExpression::TightenIndexSpaceArgs args(this, this); if (!this->realm_index_space_ready.has_triggered()) { if (!valid_event.has_triggered()) @@ -1987,18 +1989,25 @@ namespace Legion { //-------------------------------------------------------------------------- template - bool IndexSpaceUnion::remove_operation(RegionTreeForest *forest) + bool IndexSpaceUnion::invalidate_operation(void) //-------------------------------------------------------------------------- { + // Make sure we only do this one time + if (this->invalidated.fetch_add(1) > 0) + return false; // Remove the parent operation from all the sub expressions for (unsigned idx = 0; idx < sub_expressions.size(); idx++) - sub_expressions[idx]->remove_parent_operation(this); - // Then remove ourselves from the tree - if (forest != NULL) - forest->remove_union_operation(this, sub_expressions); - // Remove our expression reference added by invalidate_operation - // and return true if we should be deleted - return this->remove_base_resource_ref(IS_EXPR_REF); + sub_expressions[idx]->remove_derived_operation(this); + // We were successfully removed + return true; + } + + //-------------------------------------------------------------------------- + template + void IndexSpaceUnion::remove_operation(void) + //-------------------------------------------------------------------------- + { + this->context->remove_union_operation(this, sub_expressions); } //-------------------------------------------------------------------------- @@ -2010,6 +2019,8 @@ namespace Legion { sub_expressions(to_inter) //-------------------------------------------------------------------------- { + // Add an resource ref that will be removed by the OperationCreator + this->add_base_resource_ref(REGION_TREE_REF); std::set preconditions; std::vector > spaces(sub_expressions.size()); for (unsigned idx = 0; idx < sub_expressions.size(); idx++) @@ -2019,7 +2030,7 @@ namespace Legion { assert(sub->get_canonical_expression(this->context) == sub); #endif // Add the parent and the reference - sub->add_parent_operation(this); + sub->add_derived_operation(this); sub->add_tree_expression_reference(this->did); ApEvent precondition = sub->get_expr_index_space( &spaces[idx], this->type_tag, false/*need tight result*/); @@ -2042,7 +2053,7 @@ namespace Legion { if (!this->realm_index_space_ready.has_triggered() || !valid_event.has_triggered()) { - IndexSpaceExpression::TightenIndexSpaceArgs args(this); + IndexSpaceExpression::TightenIndexSpaceArgs args(this, this); if (!this->realm_index_space_ready.has_triggered()) { if (!valid_event.has_triggered()) @@ -2130,19 +2141,25 @@ namespace Legion { //-------------------------------------------------------------------------- template - bool IndexSpaceIntersection::remove_operation( - RegionTreeForest *forest) + bool IndexSpaceIntersection::invalidate_operation(void) //-------------------------------------------------------------------------- { + // Make sure we only do this one time + if (this->invalidated.fetch_add(1) > 0) + return false; // Remove the parent operation from all the sub expressions for (unsigned idx = 0; idx < sub_expressions.size(); idx++) - sub_expressions[idx]->remove_parent_operation(this); - // Then remove ourselves from the tree - if (forest != NULL) - forest->remove_intersection_operation(this, sub_expressions); - // Remove our expression reference added by invalidate_operation - // and return true if we should be deleted - return this->remove_base_resource_ref(IS_EXPR_REF); + sub_expressions[idx]->remove_derived_operation(this); + // We were successfully removed + return true; + } + + //-------------------------------------------------------------------------- + template + void IndexSpaceIntersection::remove_operation(void) + //-------------------------------------------------------------------------- + { + this->context->remove_intersection_operation(this, sub_expressions); } //-------------------------------------------------------------------------- @@ -2153,6 +2170,8 @@ namespace Legion { , lhs(l), rhs(r) //-------------------------------------------------------------------------- { + // Add an resource ref that will be removed by the OperationCreator + this->add_base_resource_ref(REGION_TREE_REF); #ifdef DEBUG_LEGION assert(lhs->get_canonical_expression(this->context) == lhs); assert(rhs->get_canonical_expression(this->context) == rhs); @@ -2160,7 +2179,7 @@ namespace Legion { if (lhs == rhs) { // Special case for when the expressions are the same - lhs->add_parent_operation(this); + lhs->add_derived_operation(this); lhs->add_tree_expression_reference(this->did); this->realm_index_space = Realm::IndexSpace::make_empty(); this->tight_index_space = Realm::IndexSpace::make_empty(); @@ -2171,8 +2190,8 @@ namespace Legion { { Realm::IndexSpace lhs_space, rhs_space; // Add the parent and the references - lhs->add_parent_operation(this); - rhs->add_parent_operation(this); + lhs->add_derived_operation(this); + rhs->add_derived_operation(this); lhs->add_tree_expression_reference(this->did); rhs->add_tree_expression_reference(this->did); ApEvent left_ready = @@ -2195,7 +2214,7 @@ namespace Legion { if (!this->realm_index_space_ready.has_triggered() || !valid_event.has_triggered()) { - IndexSpaceExpression::TightenIndexSpaceArgs args(this); + IndexSpaceExpression::TightenIndexSpaceArgs args(this, this); if (!this->realm_index_space_ready.has_triggered()) { if (!valid_event.has_triggered()) @@ -2282,20 +2301,28 @@ namespace Legion { //-------------------------------------------------------------------------- template - bool IndexSpaceDifference::remove_operation(RegionTreeForest *forest) + bool IndexSpaceDifference::invalidate_operation(void) //-------------------------------------------------------------------------- { + // Make sure we only do this one time + if (this->invalidated.fetch_add(1) > 0) + return false; // Remove the parent operation from all the sub expressions if (lhs != NULL) - lhs->remove_parent_operation(this); + lhs->remove_derived_operation(this); if ((rhs != NULL) && (lhs != rhs)) - rhs->remove_parent_operation(this); - // Then remove ourselves from the tree - if ((forest != NULL) && (lhs != NULL) && (rhs != NULL)) - forest->remove_subtraction_operation(this, lhs, rhs); - // Remove our expression reference added by invalidate_operation - // and return true if we should be deleted - return this->remove_base_resource_ref(IS_EXPR_REF); + rhs->remove_derived_operation(this); + // We were successfully removed + return true; + } + + //-------------------------------------------------------------------------- + template + void IndexSpaceDifference::remove_operation(void) + //-------------------------------------------------------------------------- + { + if ((lhs != NULL) && (rhs != NULL)) + this->context->remove_subtraction_operation(this, lhs, rhs); } ///////////////////////////////////////////////////////////// @@ -2310,6 +2337,11 @@ namespace Legion { IndexSpaceOperation::INSTANCE_EXPRESSION_KIND, forest) //-------------------------------------------------------------------------- { + // This is another kind of live expression made by the region tree + this->add_base_expression_reference(LIVE_EXPR_REF); + if (implicit_live_expressions == NULL) + implicit_live_expressions = new std::vector; + implicit_live_expressions->emplace_back(this); #ifdef DEBUG_LEGION assert(num_rects > 0); #endif @@ -2322,7 +2354,7 @@ namespace Legion { const RtEvent valid_event(this->realm_index_space.make_valid()); if (!valid_event.has_triggered()) { - IndexSpaceExpression::TightenIndexSpaceArgs args(this); + IndexSpaceExpression::TightenIndexSpaceArgs args(this, this); this->tight_index_space_ready = forest->runtime->issue_runtime_meta_task(args, LG_LATENCY_WORK_PRIORITY, valid_event); @@ -2418,7 +2450,7 @@ namespace Legion { //-------------------------------------------------------------------------- template - bool InstanceExpression::remove_operation(RegionTreeForest *forest) + bool InstanceExpression::invalidate_operation(void) //-------------------------------------------------------------------------- { // should never be called @@ -2426,6 +2458,14 @@ namespace Legion { return false; } + //-------------------------------------------------------------------------- + template + void InstanceExpression::remove_operation(void) + //-------------------------------------------------------------------------- + { + // Nothing to do here since we're not in the region tree + } + ///////////////////////////////////////////////////////////// // Remote Expression ///////////////////////////////////////////////////////////// @@ -2481,7 +2521,7 @@ namespace Legion { //-------------------------------------------------------------------------- template - bool RemoteExpression::remove_operation(RegionTreeForest *forest) + bool RemoteExpression::invalidate_operation(void) //-------------------------------------------------------------------------- { // should never be called @@ -2489,6 +2529,15 @@ namespace Legion { return false; } + //-------------------------------------------------------------------------- + template + void RemoteExpression::remove_operation(void) + //-------------------------------------------------------------------------- + { + // should never be called + assert(false); + } + ///////////////////////////////////////////////////////////// // Templated Index Space Node ///////////////////////////////////////////////////////////// @@ -2688,7 +2737,7 @@ namespace Legion { if (!index_space_ready.has_triggered() || !valid_event.has_triggered()) { // If this index space isn't ready yet, then we have to defer this - TightenIndexSpaceArgs args(this); + TightenIndexSpaceArgs args(this, this); if (!index_space_ready.has_triggered()) { if (!valid_event.has_triggered()) diff --git a/runtime/legion/runtime.cc b/runtime/legion/runtime.cc index b34322e005..978b1c2706 100644 --- a/runtime/legion/runtime.cc +++ b/runtime/legion/runtime.cc @@ -75,6 +75,7 @@ namespace Legion { __thread AutoLock *local_lock_list = NULL; __thread UniqueID implicit_provenance = 0; __thread unsigned inside_registration_callback = NO_REGISTRATION_CALLBACK; + __thread std::vector *implicit_live_expressions=NULL; const LgEvent LgEvent::NO_LG_EVENT = LgEvent(); const ApEvent ApEvent::NO_AP_EVENT = ApEvent(); @@ -13143,11 +13144,25 @@ namespace Legion { bool Runtime::is_index_partition_disjoint(Context ctx, IndexPartition p) //-------------------------------------------------------------------------- { +#ifdef DEBUG_LEGION + assert(implicit_live_expressions == NULL); +#endif if (ctx != DUMMY_CONTEXT) ctx->begin_runtime_call(); - bool result = forest->is_index_partition_disjoint(p); + const bool result = forest->is_index_partition_disjoint(p); if (ctx != DUMMY_CONTEXT) ctx->end_runtime_call(); + else if (implicit_live_expressions != NULL) + { + // Remove references to any live index space expressions we have + for (std::vector::const_iterator it = + implicit_live_expressions->begin(); it != + implicit_live_expressions->end(); it++) + if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) + delete (*it); + delete implicit_live_expressions; + implicit_live_expressions = NULL; + } return result; } @@ -13155,18 +13170,47 @@ namespace Legion { bool Runtime::is_index_partition_disjoint(IndexPartition p) //-------------------------------------------------------------------------- { - return forest->is_index_partition_disjoint(p); +#ifdef DEBUG_LEGION + assert(implicit_live_expressions == NULL); +#endif + const bool result = forest->is_index_partition_disjoint(p); + if (implicit_live_expressions != NULL) + { + // Remove references to any live index space expressions we have + for (std::vector::const_iterator it = + implicit_live_expressions->begin(); it != + implicit_live_expressions->end(); it++) + if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) + delete (*it); + delete implicit_live_expressions; + implicit_live_expressions = NULL; + } + return result; } //-------------------------------------------------------------------------- bool Runtime::is_index_partition_complete(Context ctx, IndexPartition p) //-------------------------------------------------------------------------- { +#ifdef DEBUG_LEGION + assert(implicit_live_expressions == NULL); +#endif if (ctx != DUMMY_CONTEXT) ctx->begin_runtime_call(); bool result = forest->is_index_partition_complete(p); if (ctx != DUMMY_CONTEXT) ctx->end_runtime_call(); + else if (implicit_live_expressions != NULL) + { + // Remove references to any live index space expressions we have + for (std::vector::const_iterator it = + implicit_live_expressions->begin(); it != + implicit_live_expressions->end(); it++) + if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) + delete (*it); + delete implicit_live_expressions; + implicit_live_expressions = NULL; + } return result; } @@ -13174,7 +13218,22 @@ namespace Legion { bool Runtime::is_index_partition_complete(IndexPartition p) //-------------------------------------------------------------------------- { - return forest->is_index_partition_complete(p); +#ifdef DEBUG_LEGION + assert(implicit_live_expressions == NULL); +#endif + const bool result = forest->is_index_partition_complete(p); + if (implicit_live_expressions != NULL) + { + // Remove references to any live index space expressions we have + for (std::vector::const_iterator it = + implicit_live_expressions->begin(); it != + implicit_live_expressions->end(); it++) + if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) + delete (*it); + delete implicit_live_expressions; + implicit_live_expressions = NULL; + } + return result; } //-------------------------------------------------------------------------- @@ -24370,6 +24429,7 @@ namespace Legion { if (!runtime->local_utils.empty()) assert(implicit_context == NULL); // this better hold #endif + assert(implicit_live_expressions == NULL); #endif implicit_runtime = runtime; // We immediately bump the priority of all meta-tasks once they start @@ -25000,6 +25060,17 @@ namespace Legion { default: assert(false); // should never get here } + if (implicit_live_expressions != NULL) + { + // Remove references to any live index space expressions we have + for (std::vector::const_iterator it = + implicit_live_expressions->begin(); it != + implicit_live_expressions->end(); it++) + if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) + delete (*it); + delete implicit_live_expressions; + implicit_live_expressions = NULL; + } #ifdef DEBUG_LEGION if (tid < LG_BEGIN_SHUTDOWN_TASK_IDS) runtime->decrement_total_outstanding_tasks(tid, true/*meta*/); @@ -25077,6 +25148,7 @@ namespace Legion { Runtime *runtime = *((Runtime**)userdata); #ifdef DEBUG_LEGION assert(userlen == sizeof(Runtime**)); + assert(implicit_live_expressions == NULL); #endif implicit_runtime = runtime; // We immediately bump the priority of all meta-tasks once they start @@ -25116,6 +25188,17 @@ namespace Legion { default: assert(false); // should never get here } + if (implicit_live_expressions != NULL) + { + // Remove references to any live index space expressions we have + for (std::vector::const_iterator it = + implicit_live_expressions->begin(); it != + implicit_live_expressions->end(); it++) + if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) + delete (*it); + delete implicit_live_expressions; + implicit_live_expressions = NULL; + } #ifdef DEBUG_LEGION runtime->decrement_total_outstanding_tasks(tid, true/*meta*/); #else From f068b35c7c70ac7c0177719456abec454ddf0fe1 Mon Sep 17 00:00:00 2001 From: Mike Bauer Date: Fri, 10 Dec 2021 01:28:18 -0800 Subject: [PATCH 5/9] legion: add missing distributed id encoding --- runtime/legion/region_tree.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/legion/region_tree.cc b/runtime/legion/region_tree.cc index 249beb85c8..4eb323fd36 100644 --- a/runtime/legion/region_tree.cc +++ b/runtime/legion/region_tree.cc @@ -6880,8 +6880,8 @@ namespace Legion { IndexSpaceOperation::IndexSpaceOperation(TypeTag tag, OperationKind kind, RegionTreeForest *ctx) : IndexSpaceExpression(tag, ctx->runtime, inter_lock), - DistributedCollectable(ctx->runtime, - ctx->runtime->get_available_distributed_id(), + DistributedCollectable(ctx->runtime, LEGION_DISTRIBUTED_HELP_ENCODE( + ctx->runtime->get_available_distributed_id(), INDEX_EXPR_NODE_DC), ctx->runtime->address_space), context(ctx), origin_expr(this), op_kind(kind), invalidated(0) #ifdef DEBUG_LEGION From 6bc8030e1b33b5be4c326ac0eb9223b2260b1a1c Mon Sep 17 00:00:00 2001 From: Mike Bauer Date: Mon, 13 Dec 2021 17:58:51 -0800 Subject: [PATCH 6/9] legion: more work on reference counting for distributed index space expressions --- runtime/legion/garbage_collection.cc | 89 +++++++++ runtime/legion/garbage_collection.h | 43 ++++- runtime/legion/legion_analysis.cc | 87 +++++---- runtime/legion/legion_analysis.h | 20 +- runtime/legion/legion_context.cc | 2 +- runtime/legion/legion_context.h | 14 +- runtime/legion/legion_instances.cc | 93 +++++----- runtime/legion/legion_instances.h | 30 ++- runtime/legion/legion_ops.h | 3 +- runtime/legion/legion_tasks.cc | 2 +- runtime/legion/legion_types.h | 36 ++-- runtime/legion/legion_views.cc | 11 +- runtime/legion/region_tree.cc | 262 +++++++++++++++++++-------- runtime/legion/region_tree.h | 41 ++++- runtime/legion/region_tree.inl | 28 ++- runtime/legion/runtime.cc | 104 ++++------- 16 files changed, 534 insertions(+), 331 deletions(-) diff --git a/runtime/legion/garbage_collection.cc b/runtime/legion/garbage_collection.cc index 840c685068..d3b185827b 100644 --- a/runtime/legion/garbage_collection.cc +++ b/runtime/legion/garbage_collection.cc @@ -31,6 +31,7 @@ namespace Legion { //-------------------------------------------------------------------------- LocalReferenceMutator::LocalReferenceMutator( const LocalReferenceMutator &rhs) + : waiter(rhs.waiter) //-------------------------------------------------------------------------- { // should never be called @@ -43,6 +44,9 @@ namespace Legion { { if (!mutation_effects.empty()) { +#ifdef DEBUG_LEGION + assert(waiter); +#endif RtEvent wait_on = Runtime::merge_events(mutation_effects); wait_on.wait(); } @@ -69,6 +73,9 @@ namespace Legion { RtEvent LocalReferenceMutator::get_done_event(void) //-------------------------------------------------------------------------- { +#ifdef DEBUG_LEGION + assert(!waiter); +#endif if (mutation_effects.empty()) return RtEvent::NO_RT_EVENT; RtEvent result = Runtime::merge_events(mutation_effects); @@ -108,6 +115,20 @@ namespace Legion { mutation_effects.insert(ev); } + ///////////////////////////////////////////////////////////// + // ImplicitReferenceTracker + ///////////////////////////////////////////////////////////// + + //-------------------------------------------------------------------------- + ImplicitReferenceTracker::~ImplicitReferenceTracker(void) + //-------------------------------------------------------------------------- + { + for (std::vector::const_iterator it = + live_expressions.begin(); it != live_expressions.end(); it++) + if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) + delete (*it); + } + ///////////////////////////////////////////////////////////// // DistributedCollectable ///////////////////////////////////////////////////////////// @@ -1800,6 +1821,23 @@ namespace Legion { #ifdef DEBUG_LEGION assert(count != 0); assert(registered_with_runtime); +#endif +#if 0 + // If there is no mutator or it is a non-waiting mutator then we + // can buffer this up in the implicit reference tracker and send it + // at the end of the runtime call or meta-task + if ((mutator == NULL) || !mutator->is_waiting_mutator()) + { + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + const RtEvent send_event = + implicit_reference_tracker->record_valid_increment(did, target, + precondition, count); + if (mutator != NULL) + mutator->record_reference_mutation_effect( + implicit_reference_tracker->get_effects_event()); + return send_event; + } #endif RtUserEvent done_event; if (mutator != NULL) @@ -1836,6 +1874,23 @@ namespace Legion { #ifdef DEBUG_LEGION assert(count != 0); assert(registered_with_runtime); +#endif +#if 0 + // If there is no mutator or it is a non-waiting mutator then we + // can buffer this up in the implicit reference tracker and send it + // at the end of the runtime call or meta-task + if ((mutator == NULL) || !mutator->is_waiting_mutator()) + { + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + const RtEvent send_event = + implicit_reference_tracker->record_valid_decrement(did, target, + precondition, count); + if (mutator != NULL) + mutator->record_reference_mutation_effect( + implicit_reference_tracker->get_effects_event()); + return send_event; + } #endif RtUserEvent done_event; if (mutator != NULL) @@ -1872,6 +1927,23 @@ namespace Legion { #ifdef DEBUG_LEGION assert(count != 0); assert(registered_with_runtime); +#endif +#if 0 + // If there is no mutator or it is a non-waiting mutator then we + // can buffer this up in the implicit reference tracker and send it + // at the end of the runtime call or meta-task + if ((mutator == NULL) || !mutator->is_waiting_mutator()) + { + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + const RtEvent send_event = + implicit_reference_tracker->record_gc_increment(did, target, + precondition, count); + if (mutator != NULL) + mutator->record_reference_mutation_effect( + implicit_reference_tracker->get_effects_event()); + return send_event; + } #endif RtUserEvent done_event; if (mutator != NULL) @@ -1908,6 +1980,23 @@ namespace Legion { #ifdef DEBUG_LEGION assert(count != 0); assert(registered_with_runtime); +#endif +#if 0 + // If there is no mutator or it is a non-waiting mutator then we + // can buffer this up in the implicit reference tracker and send it + // at the end of the runtime call or meta-task + if ((mutator == NULL) || !mutator->is_waiting_mutator()) + { + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + const RtEvent send_event = + implicit_reference_tracker->record_gc_increment(did, target, + precondition, count); + if (mutator != NULL) + mutator->record_reference_mutation_effect( + implicit_reference_tracker->get_effects_event()); + return send_event; + } #endif RtUserEvent done_event; if (mutator != NULL) diff --git a/runtime/legion/garbage_collection.h b/runtime/legion/garbage_collection.h index 7166e010ec..ae2c09445d 100644 --- a/runtime/legion/garbage_collection.h +++ b/runtime/legion/garbage_collection.h @@ -182,6 +182,7 @@ namespace Legion { */ class ReferenceMutator { public: + virtual bool is_waiting_mutator(void) const = 0; virtual void record_reference_mutation_effect(RtEvent event) = 0; }; @@ -193,17 +194,19 @@ namespace Legion { */ class LocalReferenceMutator : public ReferenceMutator { public: - LocalReferenceMutator(void) { } + LocalReferenceMutator(bool wait) : waiter(wait) { } LocalReferenceMutator(const LocalReferenceMutator &rhs); ~LocalReferenceMutator(void); public: LocalReferenceMutator& operator=(const LocalReferenceMutator &rhs); public: + virtual bool is_waiting_mutator(void) const { return waiter; } virtual void record_reference_mutation_effect(RtEvent event); public: RtEvent get_done_event(void); private: std::set mutation_effects; + const bool waiter; }; /** @@ -219,18 +222,46 @@ namespace Legion { public: WrapperReferenceMutator& operator=(const WrapperReferenceMutator &rhs); public: + virtual bool is_waiting_mutator(void) const { return false; } virtual void record_reference_mutation_effect(RtEvent event); private: std::set &mutation_effects; }; /** - * \class IgnoreReferenceMutator - * This will ignore any reference effects + * \class ImplicitReferenceTracker + * This class tracks implicit references that are held either by + * an application runtime API call or a meta-task. At the end of the + * runtime API call or meta-task the references are updated. */ - class IgnoreReferenceMutator : public ReferenceMutator { - public: - virtual void record_reference_mutation_effect(RtEvent event) { } + class ImplicitReferenceTracker { + public: + ImplicitReferenceTracker(void) { } + ImplicitReferenceTracker(const ImplicitReferenceTracker&) = delete; + ~ImplicitReferenceTracker(void); + public: + ImplicitReferenceTracker& operator=( + const ImplicitReferenceTracker&) = delete; + public: + inline void record_live_expression(IndexSpaceExpression *expr) + { live_expressions.emplace_back(expr); } +#if 0 + public: + RtEvent record_valid_increment(DistributedID did, + AddressSpaceID target, + unsigned count); + RtEvent record_valid_decrement(DistributedID did, + AddressSpaceID target, + unsigned count); + RtEvent record_gc_increment(DistributedID did, + AddressSpaceID target, + unsigned count); + RtEvent record_gc_decrement(DistributedID did, + AddressSpaceID target, + unsigned count); +#endif + private: + std::vector live_expressions; }; /** diff --git a/runtime/legion/legion_analysis.cc b/runtime/legion/legion_analysis.cc index 90d8440167..a929dbae3f 100644 --- a/runtime/legion/legion_analysis.cc +++ b/runtime/legion/legion_analysis.cc @@ -130,8 +130,8 @@ namespace Legion { } //-------------------------------------------------------------------------- - void PhysicalUser::pack_user(Serializer &rez, - const AddressSpaceID target) const + void PhysicalUser::pack_user(Serializer &rez, const AddressSpaceID target, + bool need_reference) const //-------------------------------------------------------------------------- { RezCheck z(rez); @@ -139,12 +139,11 @@ namespace Legion { rez.serialize(collect_event); #endif rez.serialize(usage); - expr->pack_expression(rez, target); + expr->pack_expression(rez, target, need_reference); rez.serialize(op_id); rez.serialize(index); rez.serialize(copy_user); rez.serialize(covers); - } //-------------------------------------------------------------------------- @@ -10082,7 +10081,7 @@ namespace Legion { if (is_logical_owner() || initial_refinement) { // We're the owner so we can do the merge - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(false/*waiter*/); for (FieldMaskSet::const_iterator it = new_views.begin(); it != new_views.end(); it++) if (valid_instances.insert(it->first, it->second)) @@ -10611,7 +10610,7 @@ namespace Legion { transition_event = RtUserEvent::NO_RT_USER_EVENT; } eq_state = MAPPING_STATE; - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(false/*waiter*/); // Add references to all the views that we've loaded for (FieldMaskSet::const_iterator it = valid_instances.begin(); it != valid_instances.end(); it++) @@ -13559,7 +13558,8 @@ namespace Legion { else rez.serialize(logical_owner_space); } - set_expr->pack_expression(rez, target); + // No need for a reference here since we know we'll continue holding it + set_expr->pack_expression(rez, target, false/*need reference*/); if (index_space_node != NULL) rez.serialize(index_space_node->handle); else @@ -13886,7 +13886,7 @@ namespace Legion { const RemoteRefTaskArgs *rargs = (const RemoteRefTaskArgs*)args; if (rargs->done_event.exists()) { - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(false/*waiter*/); if (rargs->add_references) { for (std::map::const_iterator it = @@ -13925,16 +13925,14 @@ namespace Legion { RayTracer *t, IndexSpaceExpression *e, IndexSpace h, AddressSpaceID o, RtUserEvent d, RtUserEvent def, const FieldMask &m, - bool local, bool is_expr_s, IndexSpace expr_h, - IndexSpaceExprID expr_i) + const PendingRemoteExpression *p) : LgTaskArgs(implicit_provenance), - set(s), target(t), expr(local ? e : NULL), handle(h), origin(o), + set(s), target(t), expr(e), handle(h), origin(o), done(d), deferral(def), ray_mask(new FieldMask(m)), - expr_handle(expr_h), expr_id(expr_i), is_local(local), - is_expr_space(is_expr_s) + pending((p == NULL) ? NULL : new PendingRemoteExpression(*p)) //-------------------------------------------------------------------------- { - if (local) + if (expr != NULL) expr->add_base_expression_reference(META_TASK_REF); } @@ -13946,18 +13944,20 @@ namespace Legion { const DeferRayTraceArgs *dargs = (const DeferRayTraceArgs*)args; // See if we need to load the expression or not - IndexSpaceExpression *expr = (dargs->is_local) ? dargs->expr : - (dargs->is_expr_space) ? runtime->forest->get_node(dargs->expr_handle) - : runtime->forest->find_remote_expression(dargs->expr_id); + IndexSpaceExpression *expr = dargs->expr; + if (expr == NULL) + expr = runtime->forest->find_remote_expression(*(dargs->pending)); dargs->set->ray_trace_equivalence_sets(dargs->target, expr, *(dargs->ray_mask), dargs->handle, dargs->origin, dargs->done, dargs->deferral); // Clean up our ray mask delete dargs->ray_mask; // Remove our expression reference too - if (dargs->is_local && + if ((dargs->expr != NULL) && dargs->expr->remove_base_expression_reference(META_TASK_REF)) delete dargs->expr; + if (dargs->pending != NULL) + delete dargs->pending; } //-------------------------------------------------------------------------- @@ -14049,14 +14049,15 @@ namespace Legion { //-------------------------------------------------------------------------- EquivalenceSet::DeferResponseArgs::DeferResponseArgs(DistributedID id, AddressSpaceID src, AddressSpaceID log, - IndexSpaceExpression *ex, bool local, bool is_space, - IndexSpace expr_h, IndexSpaceExprID xid, IndexSpace h) + IndexSpaceExpression *ex, + const PendingRemoteExpression &p, IndexSpace h) : LgTaskArgs(implicit_provenance), - did(id), source(src), logical_owner(log), expr(ex), is_local(local), - is_index_space(is_space), expr_handle(expr_h), expr_id(xid), handle(h) + did(id), source(src), logical_owner(log), expr(ex), + pending((expr != NULL) ? NULL : new PendingRemoteExpression(p)), + handle(h) //-------------------------------------------------------------------------- { - if (is_local) + if (expr != NULL) expr->add_base_expression_reference(META_TASK_REF); } @@ -14070,11 +14071,11 @@ namespace Legion { derez.deserialize(did); AddressSpaceID logical_owner; derez.deserialize(logical_owner); - bool is_local, is_index_space; - IndexSpace expr_handle; IndexSpaceExprID expr_id; RtEvent wait_for; + PendingRemoteExpression pending; + RtEvent wait_for; IndexSpaceExpression *expr = - IndexSpaceExpression::unpack_expression(derez, runtime->forest, source, - is_local, is_index_space, expr_handle, expr_id, wait_for); + IndexSpaceExpression::unpack_expression(derez, runtime->forest, + source, pending, wait_for); IndexSpace handle; derez.deserialize(handle); IndexSpaceNode *node = NULL; RtEvent wait_on; @@ -14096,16 +14097,15 @@ namespace Legion { precondition = wait_on; if (precondition.exists() && !precondition.has_triggered()) { - DeferResponseArgs args(did, source, logical_owner, expr, is_local, - is_index_space, expr_handle, expr_id, handle); + DeferResponseArgs args(did, source, logical_owner, expr, + pending, handle); runtime->issue_runtime_meta_task(args, LG_LATENCY_MESSAGE_PRIORITY, precondition); return; } // If we fall through we need to refetch things that we didn't get if (expr == NULL) - expr = is_index_space ? runtime->forest->get_node(expr_handle) : - runtime->forest->find_remote_expression(expr_id); + expr = runtime->forest->find_remote_expression(pending); if (handle.exists() && (node == NULL)) node = runtime->forest->get_node(handle); } @@ -14128,9 +14128,9 @@ namespace Legion { //-------------------------------------------------------------------------- { const DeferResponseArgs *dargs = (const DeferResponseArgs*)args; - IndexSpaceExpression *expr = (dargs->is_local) ? dargs->expr : - (dargs->is_index_space) ? runtime->forest->get_node(dargs->expr_handle) - : runtime->forest->find_remote_expression(dargs->expr_id); + IndexSpaceExpression *expr = dargs->expr; + if (expr == NULL) + expr = runtime->forest->find_remote_expression(*(dargs->pending)); IndexSpaceNode *node = NULL; if (dargs->handle.exists() && (dargs->logical_owner == runtime->address_space)) @@ -14147,9 +14147,11 @@ namespace Legion { // Once construction is complete then we do the registration set->register_with_runtime(NULL/*no remote registration needed*/); // Remove our expression reference too - if (dargs->is_local && + if ((dargs->expr != NULL) && dargs->expr->remove_base_expression_reference(META_TASK_REF)) delete dargs->expr; + if (dargs->pending != NULL) + delete dargs->pending; } //-------------------------------------------------------------------------- @@ -14233,13 +14235,11 @@ namespace Legion { RayTracer *target; derez.deserialize(target); - bool is_local, is_expr_space; - IndexSpace expr_handle; - IndexSpaceExprID expr_id; + PendingRemoteExpression pending; RtEvent expr_ready; IndexSpaceExpression *expr = - IndexSpaceExpression::unpack_expression(derez, runtime->forest, source, - is_local, is_expr_space, expr_handle, expr_id, expr_ready); + IndexSpaceExpression::unpack_expression(derez, runtime->forest, + source, pending, expr_ready); FieldMask ray_mask; derez.deserialize(ray_mask); IndexSpace handle; @@ -14257,15 +14257,13 @@ namespace Legion { DeferRayTraceArgs args(set, target, expr, handle, origin, done_event, RtUserEvent::NO_RT_USER_EVENT, - ray_mask, is_local, is_expr_space, - expr_handle, expr_id); + ray_mask, &pending); runtime->issue_runtime_meta_task(args, LG_THROUGHPUT_DEFERRED_PRIORITY, defer); return; } if (expr_ready.exists()) - expr = (is_expr_space) ? runtime->forest->get_node(expr_handle) : - runtime->forest->find_remote_expression(expr_id); + expr = runtime->forest->find_remote_expression(pending); // Fall through and actually do the operation now } set->ray_trace_equivalence_sets(target, expr, ray_mask, handle, @@ -14314,7 +14312,6 @@ namespace Legion { RtUserEvent done; derez.deserialize(done); - LocalReferenceMutator mutator; if (ready.exists() && !ready.has_triggered()) ready.wait(); set->unpack_migration(derez, source, done); diff --git a/runtime/legion/legion_analysis.h b/runtime/legion/legion_analysis.h index 58c2205b1e..116d605d78 100644 --- a/runtime/legion/legion_analysis.h +++ b/runtime/legion/legion_analysis.h @@ -705,7 +705,8 @@ namespace Legion { public: PhysicalUser& operator=(const PhysicalUser &rhs); public: - void pack_user(Serializer &rez, const AddressSpaceID target) const; + void pack_user(Serializer &rez, const AddressSpaceID target, + bool need_reference = true) const; static PhysicalUser* unpack_user(Deserializer &derez, RegionTreeForest *forest, const AddressSpaceID source); public: @@ -2120,9 +2121,7 @@ namespace Legion { // These are just for the case where the // request comes from a remote node and // we're waiting for the expression to load - bool is_local=true, bool is_expr_s = false, - IndexSpace expr_h = IndexSpace::NO_SPACE, - IndexSpaceExprID expr_i = 0); + const PendingRemoteExpression *pending = NULL); public: EquivalenceSet *const set; RayTracer *const target; @@ -2132,10 +2131,7 @@ namespace Legion { const RtUserEvent done; const RtUserEvent deferral; FieldMask *const ray_mask; - const IndexSpace expr_handle; - const IndexSpaceExprID expr_id; - const bool is_local; - const bool is_expr_space; + const PendingRemoteExpression *const pending; }; struct DeferRayTraceFinishArgs : public LgTaskArgs { @@ -2213,17 +2209,13 @@ namespace Legion { public: DeferResponseArgs(DistributedID id, AddressSpaceID src, AddressSpaceID log, IndexSpaceExpression *ex, - bool local, bool is_space, IndexSpace expr_h, - IndexSpaceExprID xid, IndexSpace h); + const PendingRemoteExpression &pending, IndexSpace h); public: const DistributedID did; const AddressSpaceID source; const AddressSpaceID logical_owner; IndexSpaceExpression *const expr; - const bool is_local; - const bool is_index_space; - const IndexSpace expr_handle; - const IndexSpaceExprID expr_id; + const PendingRemoteExpression *const pending; const IndexSpace handle; }; struct DeferRemoveRefArgs : public LgTaskArgs { diff --git a/runtime/legion/legion_context.cc b/runtime/legion/legion_context.cc index 2da9ccc413..7e4fc5daf2 100644 --- a/runtime/legion/legion_context.cc +++ b/runtime/legion/legion_context.cc @@ -5785,7 +5785,7 @@ namespace Legion { const DistributedID did = runtime->get_available_distributed_id(); FutureMapImpl *impl = new FutureMapImpl(this, runtime, did, runtime->address_space, RtEvent::NO_RT_EVENT); - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(true/*waiter*/); for (std::map::const_iterator it = data.begin(); it != data.end(); it++) { diff --git a/runtime/legion/legion_context.h b/runtime/legion/legion_context.h index ee2d2b16cb..8e6452579f 100644 --- a/runtime/legion/legion_context.h +++ b/runtime/legion/legion_context.h @@ -2064,7 +2064,7 @@ namespace Legion { //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION - assert(implicit_live_expressions == NULL); + assert(implicit_reference_tracker == NULL); #endif if (overhead_tracker == NULL) return; @@ -2078,16 +2078,10 @@ namespace Legion { inline void TaskContext::end_runtime_call(void) //-------------------------------------------------------------------------- { - if (implicit_live_expressions != NULL) + if (implicit_reference_tracker != NULL) { - // Remove references to any live index space expressions we have - for (std::vector::const_iterator it = - implicit_live_expressions->begin(); it != - implicit_live_expressions->end(); it++) - if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) - delete (*it); - delete implicit_live_expressions; - implicit_live_expressions = NULL; + delete implicit_reference_tracker; + implicit_reference_tracker = NULL; } if (overhead_tracker == NULL) return; diff --git a/runtime/legion/legion_instances.cc b/runtime/legion/legion_instances.cc index f6ccf78777..83be9c6656 100644 --- a/runtime/legion/legion_instances.cc +++ b/runtime/legion/legion_instances.cc @@ -1479,7 +1479,8 @@ namespace Legion { rez.serialize(memory_manager->memory); rez.serialize(instance); rez.serialize(instance_footprint); - instance_domain->pack_expression(rez, target); + // No need for a reference here since we know we'll continue holding it + instance_domain->pack_expression(rez, target, false/*need reference*/); rez.serialize(piece_list_size); if (piece_list_size > 0) rez.serialize(piece_list, piece_list_size); @@ -1510,13 +1511,11 @@ namespace Legion { derez.deserialize(inst); size_t inst_footprint; derez.deserialize(inst_footprint); - bool local_is, domain_is; - IndexSpace domain_handle; - IndexSpaceExprID domain_expr_id; + PendingRemoteExpression pending; RtEvent domain_ready; IndexSpaceExpression *inst_domain = IndexSpaceExpression::unpack_expression(derez, runtime->forest, source, - local_is, domain_is, domain_handle, domain_expr_id, domain_ready); + pending, domain_ready); size_t piece_list_size; derez.deserialize(piece_list_size); void *piece_list = NULL; @@ -1551,18 +1550,16 @@ namespace Legion { { // We need to defer this instance creation DeferIndividualManagerArgs args(did, owner_space, mem, inst, - inst_footprint, local_is, inst_domain, domain_is, domain_handle, - domain_expr_id, handle, tree_id, layout_id, unique_event, redop, - piece_list, piece_list_size, shadow_inst); + inst_footprint, inst_domain, pending, + handle, tree_id, layout_id, unique_event, redop, + piece_list, piece_list_size, source, shadow_inst); runtime->issue_runtime_meta_task(args, LG_LATENCY_RESPONSE_PRIORITY, precondition); return; } // If we fall through we need to refetch things that we didn't get if (domain_ready.exists()) - inst_domain = domain_is ? - runtime->forest->get_node(domain_handle) : - runtime->forest->find_remote_expression(domain_expr_id); + inst_domain = runtime->forest->find_remote_expression(pending); if (fs_ready.exists()) space_node = runtime->forest->get_node(handle); if (layout_ready.exists()) @@ -1579,19 +1576,18 @@ namespace Legion { //-------------------------------------------------------------------------- IndividualManager::DeferIndividualManagerArgs::DeferIndividualManagerArgs( DistributedID d, AddressSpaceID own, Memory m, PhysicalInstance i, - size_t f, bool local, IndexSpaceExpression *lx, bool is, - IndexSpace dh, IndexSpaceExprID dx, FieldSpace h, RegionTreeID tid, + size_t f, IndexSpaceExpression *lx, + const PendingRemoteExpression &p, FieldSpace h, RegionTreeID tid, LayoutConstraintID l, ApEvent u, ReductionOpID r, const void *pl, - size_t pl_size, bool shadow) + size_t pl_size, AddressSpaceID src, bool shadow) : LgTaskArgs(implicit_provenance), - did(d), owner(own), mem(m), inst(i), footprint(f), local_is(local), - domain_is(is), local_expr(local ? lx : NULL), domain_handle(dh), - domain_expr(dx), handle(h), tree_id(tid), layout_id(l), - use_event(u), redop(r), piece_list(pl), piece_list_size(pl_size), - shadow_instance(shadow) + did(d), owner(own), mem(m), inst(i), footprint(f), pending(p), + local_expr(lx), handle(h), tree_id(tid), + layout_id(l), use_event(u), redop(r), piece_list(pl), + piece_list_size(pl_size), source(src), shadow_instance(shadow) //-------------------------------------------------------------------------- { - if (local_is) + if (local_expr != NULL) local_expr->add_base_expression_reference(META_TASK_REF); } @@ -1602,9 +1598,9 @@ namespace Legion { { const DeferIndividualManagerArgs *dargs = (const DeferIndividualManagerArgs*)args; - IndexSpaceExpression *inst_domain = dargs->local_is ? dargs->local_expr : - dargs->domain_is ? runtime->forest->get_node(dargs->domain_handle) : - runtime->forest->find_remote_expression(dargs->domain_expr); + IndexSpaceExpression *inst_domain = dargs->local_expr; + if (inst_domain == NULL) + inst_domain = runtime->forest->find_remote_expression(dargs->pending); FieldSpaceNode *space_node = runtime->forest->get_node(dargs->handle); LayoutConstraints *constraints = runtime->find_layout_constraints(dargs->layout_id); @@ -1613,7 +1609,7 @@ namespace Legion { dargs->piece_list_size, space_node, dargs->tree_id, constraints, dargs->use_event, dargs->redop, dargs->shadow_instance); // Remove the local expression reference if necessary - if (dargs->local_is && + if ((dargs->local_expr != NULL) && dargs->local_expr->remove_base_expression_reference(META_TASK_REF)) delete dargs->local_expr; } @@ -2905,7 +2901,8 @@ namespace Legion { rez.serialize(owner_space); rez.serialize(point_space->handle); rez.serialize(instance_footprint); - instance_domain->pack_expression(rez, target); + // No need for a reference here since we know we'll continue holding it + instance_domain->pack_expression(rez, target, false/*need reference*/); rez.serialize(field_space_node->handle); rez.serialize(tree_id); rez.serialize(redop); @@ -2933,13 +2930,11 @@ namespace Legion { runtime->forest->get_node(points_handle, &points_ready); size_t inst_footprint; derez.deserialize(inst_footprint); - bool local_is, domain_is; - IndexSpace domain_handle; - IndexSpaceExprID domain_expr_id; + PendingRemoteExpression pending; RtEvent domain_ready; IndexSpaceExpression *inst_domain = IndexSpaceExpression::unpack_expression(derez, runtime->forest, source, - local_is, domain_is, domain_handle, domain_expr_id, domain_ready); + pending, domain_ready); size_t piece_list_size; derez.deserialize(piece_list_size); void *piece_list = NULL; @@ -2981,9 +2976,8 @@ namespace Legion { { // We need to defer this instance creation DeferCollectiveManagerArgs args(did, owner_space, points_handle, - inst_footprint, local_is, inst_domain, domain_is, domain_handle, - domain_expr_id, handle, tree_id, layout_id, unique_event, redop, - piece_list, piece_list_size); + inst_footprint, inst_domain, pending, handle, tree_id, layout_id, + unique_event, redop, piece_list, piece_list_size, source); runtime->issue_runtime_meta_task(args, LG_LATENCY_RESPONSE_PRIORITY, precondition); return; @@ -2992,9 +2986,7 @@ namespace Legion { if (points_ready.exists()) point_space = runtime->forest->get_node(points_handle); if (domain_ready.exists()) - inst_domain = domain_is ? - runtime->forest->get_node(domain_handle) : - runtime->forest->find_remote_expression(domain_expr_id); + inst_domain = runtime->forest->find_remote_expression(pending); if (fs_ready.exists()) space_node = runtime->forest->get_node(handle); if (layout_ready.exists()) @@ -3010,18 +3002,17 @@ namespace Legion { //-------------------------------------------------------------------------- CollectiveManager::DeferCollectiveManagerArgs::DeferCollectiveManagerArgs( DistributedID d, AddressSpaceID own, IndexSpace points, - size_t f, bool local, IndexSpaceExpression *lx, bool is, - IndexSpace dh, IndexSpaceExprID dx, FieldSpace h, RegionTreeID tid, + size_t f, IndexSpaceExpression *lx, + const PendingRemoteExpression &p, FieldSpace h, RegionTreeID tid, LayoutConstraintID l, ApEvent use, ReductionOpID r, - const void *pl, size_t pl_size) + const void *pl, size_t pl_size, AddressSpace src) : LgTaskArgs(implicit_provenance), - did(d), owner(own), point_space(points), footprint(f), local_is(local), - domain_is(is), local_expr(lx), domain_handle(dh), domain_expr(dx), - handle(h), tree_id(tid), layout_id(l), use_event(use), redop(r), - piece_list(pl), piece_list_size(pl_size) + did(d), owner(own), point_space(points), footprint(f), local_expr(lx), + pending(p), handle(h), tree_id(tid), layout_id(l), use_event(use), + redop(r), piece_list(pl), piece_list_size(pl_size), source(src) //-------------------------------------------------------------------------- { - if (local_is) + if (local_expr != NULL) local_expr->add_base_expression_reference(META_TASK_REF); } @@ -3034,9 +3025,9 @@ namespace Legion { (const DeferCollectiveManagerArgs*)args; IndexSpaceNode *point_space = runtime->forest->get_node(dargs->point_space); - IndexSpaceExpression *inst_domain = dargs->local_is ? dargs->local_expr : - dargs->domain_is ? runtime->forest->get_node(dargs->domain_handle) : - runtime->forest->find_remote_expression(dargs->domain_expr); + IndexSpaceExpression *inst_domain = dargs->local_expr; + if (inst_domain == NULL) + inst_domain = runtime->forest->find_remote_expression(dargs->pending); FieldSpaceNode *space_node = runtime->forest->get_node(dargs->handle); LayoutConstraints *constraints = runtime->find_layout_constraints(dargs->layout_id); @@ -3045,7 +3036,7 @@ namespace Legion { dargs->piece_list_size, space_node, dargs->tree_id, constraints, dargs->use_event, dargs->redop); // Remove the local expression reference if necessary - if (dargs->local_is && + if ((dargs->local_expr != NULL) && dargs->local_expr->remove_base_expression_reference(META_TASK_REF)) delete dargs->local_expr; } @@ -3102,7 +3093,7 @@ namespace Legion { { RtUserEvent to_trigger; derez.deserialize(to_trigger); - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(false/*waiter*/);; manager->activate_collective(&mutator); Runtime::trigger_event(to_trigger, mutator.get_done_event()); break; @@ -3111,7 +3102,7 @@ namespace Legion { { RtUserEvent to_trigger; derez.deserialize(to_trigger); - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(false/*waiter*/); manager->deactivate_collective(&mutator); Runtime::trigger_event(to_trigger, mutator.get_done_event()); break; @@ -3120,7 +3111,7 @@ namespace Legion { { RtUserEvent to_trigger; derez.deserialize(to_trigger); - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(false/*waiter*/); manager->validate_collective(&mutator); Runtime::trigger_event(to_trigger, mutator.get_done_event()); break; @@ -3129,7 +3120,7 @@ namespace Legion { { RtUserEvent to_trigger; derez.deserialize(to_trigger); - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(false/*waiter*/); manager->invalidate_collective(&mutator); Runtime::trigger_event(to_trigger, mutator.get_done_event()); break; diff --git a/runtime/legion/legion_instances.h b/runtime/legion/legion_instances.h index 7815537193..6460916253 100644 --- a/runtime/legion/legion_instances.h +++ b/runtime/legion/legion_instances.h @@ -371,23 +371,19 @@ namespace Legion { static const LgTaskID TASK_ID = LG_DEFER_INDIVIDUAL_MANAGER_TASK_ID; public: DeferIndividualManagerArgs(DistributedID d, AddressSpaceID own, - Memory m, PhysicalInstance i, size_t f, bool local, - IndexSpaceExpression *lx, bool is, IndexSpace dh, - IndexSpaceExprID dx, FieldSpace h, RegionTreeID tid, - LayoutConstraintID l, ApEvent use, ReductionOpID redop, - const void *piece_list, size_t piece_list_size, - bool shadow_instance); + Memory m, PhysicalInstance i, size_t f, IndexSpaceExpression *lx, + const PendingRemoteExpression &pending, FieldSpace h, + RegionTreeID tid, LayoutConstraintID l, ApEvent use, + ReductionOpID redop, const void *piece_list, size_t piece_list_size, + AddressSpaceID src, bool shadow_instance); public: const DistributedID did; const AddressSpaceID owner; const Memory mem; const PhysicalInstance inst; const size_t footprint; - const bool local_is; - const bool domain_is; + const PendingRemoteExpression pending; IndexSpaceExpression *local_expr; - const IndexSpace domain_handle; - const IndexSpaceExprID domain_expr; const FieldSpace handle; const RegionTreeID tree_id; const LayoutConstraintID layout_id; @@ -395,6 +391,7 @@ namespace Legion { const ReductionOpID redop; const void *const piece_list; const size_t piece_list_size; + const AddressSpaceID source; const bool shadow_instance; }; public: @@ -527,20 +524,18 @@ namespace Legion { static const LgTaskID TASK_ID = LG_DEFER_COLLECTIVE_MANAGER_TASK_ID; public: DeferCollectiveManagerArgs(DistributedID d, AddressSpaceID own, - IndexSpace p, size_t f, bool local, IndexSpaceExpression *lx, - bool is, IndexSpace dh, IndexSpaceExprID dx, FieldSpace h, + IndexSpace p, size_t f, IndexSpaceExpression *lx, + const PendingRemoteExpression &pending, FieldSpace h, RegionTreeID tid, LayoutConstraintID l, ApEvent use, - ReductionOpID redop, const void *piece_list,size_t piece_list_size); + ReductionOpID redop, const void *piece_list, + size_t piece_list_size, AddressSpaceID source); public: const DistributedID did; const AddressSpaceID owner; IndexSpace point_space; const size_t footprint; - const bool local_is; - const bool domain_is; IndexSpaceExpression *const local_expr; - const IndexSpace domain_handle; - const IndexSpaceExprID domain_expr; + const PendingRemoteExpression pending; const FieldSpace handle; const RegionTreeID tree_id; const LayoutConstraintID layout_id; @@ -548,6 +543,7 @@ namespace Legion { const ReductionOpID redop; const void *const piece_list; const size_t piece_list_size; + const AddressSpaceID source; }; public: CollectiveManager(RegionTreeForest *ctx, DistributedID did, diff --git a/runtime/legion/legion_ops.h b/runtime/legion/legion_ops.h index 4f61311fc0..23fc1e8ea2 100644 --- a/runtime/legion/legion_ops.h +++ b/runtime/legion/legion_ops.h @@ -348,6 +348,7 @@ namespace Legion { const std::vector *dependences = NULL); public: // Inherited from ReferenceMutator + virtual bool is_waiting_mutator(void) const { return false; } virtual void record_reference_mutation_effect(RtEvent event); public: RtEvent execute_prepipeline_stage(GenerationID gen, @@ -617,7 +618,7 @@ namespace Legion { protected: static inline void add_launch_space_reference(IndexSpaceNode *node) { - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(true/*waiter*/); node->add_base_valid_ref(CONTEXT_REF, &mutator); } static inline bool remove_launch_space_reference(IndexSpaceNode *node) diff --git a/runtime/legion/legion_tasks.cc b/runtime/legion/legion_tasks.cc index e7b3d69192..7d7b4f3345 100644 --- a/runtime/legion/legion_tasks.cc +++ b/runtime/legion/legion_tasks.cc @@ -9473,7 +9473,7 @@ namespace Legion { #ifdef DEBUG_LEGION assert(finder != future_handles->handles.end()); #endif - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(false/*not waiting*/); FutureImpl *impl = runtime->find_or_create_future(finder->second, parent_ctx->get_context_uid(), &mutator); if (functor != NULL) diff --git a/runtime/legion/legion_types.h b/runtime/legion/legion_types.h index d152d4f767..cdfc372de4 100644 --- a/runtime/legion/legion_types.h +++ b/runtime/legion/legion_types.h @@ -1553,7 +1553,7 @@ namespace Legion { class Notifiable; class ReferenceMutator; class LocalReferenceMutator; - class NeverReferenceMutator; + class ImplicitReferenceTracker; class DistributedCollectable; class LayoutDescription; class InstanceManager; // base class for all instances @@ -1590,6 +1590,7 @@ namespace Legion { class TreeClose; struct CloseInfo; struct FieldDataDescriptor; + struct PendingRemoteExpression; // legion_spy.h class TreeStateLogger; @@ -1629,9 +1630,10 @@ namespace Legion { // This data structure tracks references to any live // temporary index space expressions that have been // handed back by the region tree inside the execution - // of a meta-task or a runtime API call - extern __thread - std::vector *implicit_live_expressions; + // of a meta-task or a runtime API call. It also tracks + // changes to remote distributed collectable that can be + // delayed and batched together. + extern __thread ImplicitReferenceTracker *implicit_reference_tracker; /** * \class LgTaskArgs @@ -2384,11 +2386,10 @@ namespace Legion { UniqueID local_provenance = Internal::implicit_provenance; // Save whether we are in a registration callback unsigned local_callback = Internal::inside_registration_callback; - // Save any local live expressions that we have - std::vector *local_live_expressions = - Internal::implicit_live_expressions; + // Save the reference tracker that we have + ImplicitReferenceTracker *local_tracker = implicit_reference_tracker; #ifdef DEBUG_LEGION - Internal::implicit_live_expressions = NULL; + Internal::implicit_reference_tracker = NULL; #endif // Check to see if we have any local locks to notify if (Internal::local_lock_list != NULL) @@ -2429,10 +2430,10 @@ namespace Legion { // Write the registration callback information back Internal::inside_registration_callback = local_callback; #ifdef DEBUG_LEGION - assert(Internal::implicit_live_expressions == NULL); + assert(Internal::implicit_reference_tracker == NULL); #endif - // Write the local live expressions back - Internal::implicit_live_expressions = local_live_expressions; + // Write the local reference tracker back + Internal::implicit_reference_tracker = local_tracker; } //-------------------------------------------------------------------------- @@ -2445,11 +2446,10 @@ namespace Legion { UniqueID local_provenance = Internal::implicit_provenance; // Save whether we are in a registration callback unsigned local_callback = Internal::inside_registration_callback; - // Save any local live expressions that we have - std::vector *local_live_expressions = - Internal::implicit_live_expressions; + // Save the reference tracker that we have + ImplicitReferenceTracker *local_tracker = implicit_reference_tracker; #ifdef DEBUG_LEGION - Internal::implicit_live_expressions = NULL; + Internal::implicit_reference_tracker = NULL; #endif // Check to see if we have any local locks to notify if (Internal::local_lock_list != NULL) @@ -2490,10 +2490,10 @@ namespace Legion { // Write the registration callback information back Internal::inside_registration_callback = local_callback; #ifdef DEBUG_LEGION - assert(Internal::implicit_live_expressions == NULL); + assert(Internal::implicit_reference_tracker == NULL); #endif - // Write the local live expressions back - Internal::implicit_live_expressions = local_live_expressions; + // Write the local reference tracker back + Internal::implicit_reference_tracker = local_tracker; } #ifdef LEGION_SPY diff --git a/runtime/legion/legion_views.cc b/runtime/legion/legion_views.cc index b1750141d7..4a6face762 100644 --- a/runtime/legion/legion_views.cc +++ b/runtime/legion/legion_views.cc @@ -1580,7 +1580,9 @@ namespace Legion { { const unsigned index = indexes.size(); rez.serialize(index); - it->first->pack_user(rez, target); + // No need for a reference since we're replicating so we know + // that we'll continue holding this reference + it->first->pack_user(rez, target, false/*need reference*/); indexes[it->first] = index; } else @@ -1621,7 +1623,9 @@ namespace Legion { { const unsigned index = indexes.size(); rez.serialize(index); - it->first->pack_user(rez, target); + // No need for a reference since we're replicating so we know + // that we'll continue holding the reference to keep it alive + it->first->pack_user(rez, target, false/*need reference*/); indexes[it->first] = index; } else @@ -1646,7 +1650,8 @@ namespace Legion { for (FieldMaskSet::const_iterator it = needed_subviews.begin(); it != needed_subviews.end(); it++) { - it->first->view_expr->pack_expression(rez, target); + // No need for the reference since we're replicating this + it->first->view_expr->pack_expression(rez, target, false/*need ref*/); rez.serialize(it->second); it->first->pack_replication(rez, indexes, it->second, target); } diff --git a/runtime/legion/region_tree.cc b/runtime/legion/region_tree.cc index 4eb323fd36..a8d096ae74 100644 --- a/runtime/legion/region_tree.cc +++ b/runtime/legion/region_tree.cc @@ -5631,15 +5631,15 @@ namespace Legion { // Add the live reference if (mutator == NULL) { - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(true/*waiter*/); result->add_base_expression_reference(LIVE_EXPR_REF, &local_mutator); } else result->add_base_expression_reference(LIVE_EXPR_REF, mutator); // Save it in the implicit live expression references - if (implicit_live_expressions == NULL) - implicit_live_expressions = new std::vector(); - implicit_live_expressions->emplace_back(result); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); // Remove the gc reference that comes back from finding it in the tree if (result->remove_live_reference(REGION_TREE_REF)) assert(false); // should never hit this @@ -5671,7 +5671,7 @@ namespace Legion { } if (expressions.empty()) return *(exprs.begin()); - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(true/*waiter*/); if (expressions.size() == 1) { IndexSpaceExpression *result = expressions.back(); @@ -5681,10 +5681,9 @@ namespace Legion { result->add_base_expression_reference(LIVE_EXPR_REF,&local_mutator); else result->add_base_expression_reference(LIVE_EXPR_REF, mutator); - if (implicit_live_expressions == NULL) - implicit_live_expressions = - new std::vector; - implicit_live_expressions->emplace_back(result); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); } return result; } @@ -5709,10 +5708,9 @@ namespace Legion { &local_mutator); else result->add_base_expression_reference(LIVE_EXPR_REF, mutator); - if (implicit_live_expressions == NULL) - implicit_live_expressions = - new std::vector; - implicit_live_expressions->emplace_back(result); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); } return expressions.back(); } @@ -5800,10 +5798,9 @@ namespace Legion { &local_mutator); else result->add_base_expression_reference(LIVE_EXPR_REF, mutator); - if (implicit_live_expressions == NULL) - implicit_live_expressions = - new std::vector(); - implicit_live_expressions->emplace_back(result); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); } // Remove the extra expression reference we added if (result->remove_base_expression_reference(REGION_TREE_REF)) @@ -5824,9 +5821,9 @@ namespace Legion { result->add_base_expression_reference(LIVE_EXPR_REF,&local_mutator); else result->add_base_expression_reference(LIVE_EXPR_REF, mutator); - if (implicit_live_expressions == NULL) - implicit_live_expressions = new std::vector(); - implicit_live_expressions->emplace_back(result); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); } // Remove the reference added by the trie traversal if (result->remove_live_reference(REGION_TREE_REF)) @@ -5953,15 +5950,15 @@ namespace Legion { // Add the live reference if (mutator == NULL) { - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(true/*waiter*/); result->add_base_expression_reference(LIVE_EXPR_REF, &local_mutator); } else result->add_base_expression_reference(LIVE_EXPR_REF, mutator); // Save it in the implicit live expression references - if (implicit_live_expressions == NULL) - implicit_live_expressions = new std::vector(); - implicit_live_expressions->emplace_back(result); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); // Remove the gc reference that comes back with the trie traversal if (result->remove_live_reference(REGION_TREE_REF)) assert(false); // should never hit this @@ -5997,7 +5994,7 @@ namespace Legion { // remove duplicates std::vector::iterator last = std::unique(expressions.begin(), expressions.end()); - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(true/*waiter*/); if (last != expressions.end()) { expressions.erase(last, expressions.end()); @@ -6014,10 +6011,9 @@ namespace Legion { &local_mutator); else result->add_base_expression_reference(LIVE_EXPR_REF, mutator); - if (implicit_live_expressions == NULL) - implicit_live_expressions = - new std::vector; - implicit_live_expressions->emplace_back(result); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); } return result; } @@ -6092,10 +6088,9 @@ namespace Legion { &local_mutator); else unique->add_base_expression_reference(LIVE_EXPR_REF, mutator); - if (implicit_live_expressions == NULL) - implicit_live_expressions = - new std::vector; - implicit_live_expressions->emplace_back(unique); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(unique); } // Remove references on all the things we no longer need for (std::set:: @@ -6133,10 +6128,9 @@ namespace Legion { &local_mutator); else result->add_base_expression_reference(LIVE_EXPR_REF, mutator); - if (implicit_live_expressions == NULL) - implicit_live_expressions = - new std::vector(); - implicit_live_expressions->emplace_back(result); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); } // Remove the extra expression reference we added if (result->remove_base_expression_reference(REGION_TREE_REF)) @@ -6157,9 +6151,9 @@ namespace Legion { result->add_base_expression_reference(LIVE_EXPR_REF,&local_mutator); else result->add_base_expression_reference(LIVE_EXPR_REF, mutator); - if (implicit_live_expressions == NULL) - implicit_live_expressions = new std::vector; - implicit_live_expressions->emplace_back(result); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); } // Remove the reference added by the trie traversal if (result->remove_live_reference(REGION_TREE_REF)) @@ -6355,14 +6349,14 @@ namespace Legion { } if (mutator == NULL) { - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(true/*waiter*/); result->add_base_expression_reference(LIVE_EXPR_REF, &local_mutator); } else result->add_base_expression_reference(LIVE_EXPR_REF, mutator); - if (implicit_live_expressions == NULL) - implicit_live_expressions = new std::vector; - implicit_live_expressions->emplace_back(result); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); // Remove the gc reference that comes back from finding it in the tree if (result->remove_live_reference(REGION_TREE_REF)) assert(false); // should never hit this @@ -6516,8 +6510,9 @@ namespace Legion { } const AddressSpaceID owner = IndexSpaceExpression::get_owner_space(remote_expr_id, runtime); - if (owner == runtime->address_space) - return origin; +#ifdef DEBUG_LEGION + assert(owner != runtime->address_space); +#endif // Retake the lock in exclusive mode and see if we lost the race RtEvent wait_on; RtUserEvent request_event; @@ -6572,16 +6567,38 @@ namespace Legion { //-------------------------------------------------------------------------- IndexSpaceExpression* RegionTreeForest::find_remote_expression( - IndexSpaceExprID remote_expr_id) + const PendingRemoteExpression &pending) //-------------------------------------------------------------------------- { - AutoLock l_lock(lookup_is_op_lock, 1, false/*exclusive*/); - std::map::const_iterator - finder = remote_expressions.find(remote_expr_id); + if (pending.is_index_space) + return get_node(pending.handle); + IndexSpaceExpression *result = NULL; + { + AutoLock l_lock(lookup_is_op_lock, 1, false/*exclusive*/); + std::map::const_iterator + finder = remote_expressions.find(pending.remote_expr_id); #ifdef DEBUG_LEGION - assert(finder != remote_expressions.end()); + assert(finder != remote_expressions.end()); #endif - return finder->second; + result = finder->second; + } + if (pending.has_reference) + { +#ifdef DEBUG_LEGION + IndexSpaceOperation *op = dynamic_cast(result); + assert(op != NULL); +#else + IndexSpaceOperation *op = static_cast(result); +#endif + LocalReferenceMutator mutator(false/*waiter*/); + result->add_base_expression_reference(LIVE_EXPR_REF, &mutator); + op->send_remote_valid_decrement(pending.source, NULL/*mutator*/, + mutator.get_done_event()); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); + } + return result; } //-------------------------------------------------------------------------- @@ -6592,10 +6609,8 @@ namespace Legion { AutoLock l_lock(lookup_is_op_lock); std::map::iterator finder = remote_expressions.find(remote_expr_id); -#ifdef DEBUG_LEGION - assert(finder != remote_expressions.end()); -#endif - remote_expressions.erase(finder); + if (finder != remote_expressions.end()) + remote_expressions.erase(finder); } //-------------------------------------------------------------------------- @@ -6822,8 +6837,32 @@ namespace Legion { derez.deserialize(is_local); if (is_local) { + bool has_reference; + derez.deserialize(has_reference); IndexSpaceExpression *result; derez.deserialize(result); + if (has_reference) + { + if (source != forest->runtime->address_space) + { +#ifdef DEBUG_LEGION + IndexSpaceOperation *op = + dynamic_cast(result); + assert(op != NULL); +#else + IndexSpaceOperation *op = static_cast(result); +#endif + // Make this valid and then send the removal of the + // remote did expression + LocalReferenceMutator mutator(false/*waiter*/); + op->add_base_expression_reference(LIVE_EXPR_REF, &mutator); + op->send_remote_valid_decrement(source, NULL/*mutator*/, + mutator.get_done_event()); + } + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); + } return result; } bool is_index_space; @@ -6835,41 +6874,107 @@ namespace Legion { derez.deserialize(handle); return forest->get_node(handle); } + bool has_reference; + derez.deserialize(has_reference); IndexSpaceExprID remote_expr_id; derez.deserialize(remote_expr_id); IndexSpaceExpression *origin; derez.deserialize(origin); - return forest->find_or_request_remote_expression(remote_expr_id, origin); + IndexSpaceExpression *result = + forest->find_or_request_remote_expression(remote_expr_id, origin); + if (has_reference) + { +#ifdef DEBUG_LEGION + IndexSpaceOperation *op = dynamic_cast(result); + assert(op != NULL); +#else + IndexSpaceOperation *op = static_cast(result); +#endif + LocalReferenceMutator mutator(false/*waiter*/); + result->add_base_expression_reference(LIVE_EXPR_REF, &mutator); + op->send_remote_valid_decrement(source, NULL/*mutator*/, + mutator.get_done_event()); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); + } + return result; } //-------------------------------------------------------------------------- /*static*/ IndexSpaceExpression* IndexSpaceExpression::unpack_expression( - Deserializer &derez, RegionTreeForest *forest, - AddressSpaceID source, bool &is_local, - bool &is_index_space, IndexSpace &handle, - IndexSpaceExprID &remote_expr_id, RtEvent &wait_for) + Deserializer &derez, RegionTreeForest *forest, AddressSpaceID source, + PendingRemoteExpression &pending, RtEvent &wait_for) //-------------------------------------------------------------------------- { // Handle the special case where this is a local index space expression + bool is_local; derez.deserialize(is_local); if (is_local) { + derez.deserialize(pending.has_reference); IndexSpaceExpression *result; derez.deserialize(result); + if (pending.has_reference) + { + if (source != forest->runtime->address_space) + { +#ifdef DEBUG_LEGION + IndexSpaceOperation *op = + dynamic_cast(result); + assert(op != NULL); +#else + IndexSpaceOperation *op = static_cast(result); +#endif + // Make this valid and then send the removal of the + // remote did expression + LocalReferenceMutator mutator(false/*waiter*/); + op->add_base_expression_reference(LIVE_EXPR_REF, &mutator); + op->send_remote_valid_decrement(source, NULL/*mutator*/, + mutator.get_done_event()); + } + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); + } return result; } - derez.deserialize(is_index_space); + derez.deserialize(pending.is_index_space); // If this is an index space it is easy - if (is_index_space) + if (pending.is_index_space) { - derez.deserialize(handle); - return forest->get_node(handle, &wait_for); + derez.deserialize(pending.handle); + return forest->get_node(pending.handle, &wait_for); } - derez.deserialize(remote_expr_id); + derez.deserialize(pending.has_reference); + derez.deserialize(pending.remote_expr_id); IndexSpaceExpression *origin; derez.deserialize(origin); - return forest->find_or_request_remote_expression(remote_expr_id, - origin, &wait_for); + IndexSpaceExpression *result = + forest->find_or_request_remote_expression(pending.remote_expr_id, + origin, &wait_for); + if (result == NULL) + { + pending.source = source; + return result; + } + if (pending.has_reference) + { +#ifdef DEBUG_LEGION + IndexSpaceOperation *op = dynamic_cast(result); + assert(op != NULL); +#else + IndexSpaceOperation *op = static_cast(result); +#endif + LocalReferenceMutator mutator(false/*waiter*/); + result->add_base_expression_reference(LIVE_EXPR_REF, &mutator); + op->send_remote_valid_decrement(source, NULL/*mutator*/, + mutator.get_done_event()); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); + } + return result; } ///////////////////////////////////////////////////////////// @@ -7048,7 +7153,7 @@ namespace Legion { { if (mutator == NULL) { - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(true/*waiter*/); add_base_valid_ref(source, &local_mutator, count); } else @@ -7071,7 +7176,7 @@ namespace Legion { { if (mutator == NULL) { - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(true/*waiter*/); add_nested_valid_ref(source, &local_mutator, count); } else @@ -8573,7 +8678,7 @@ namespace Legion { // If this is above then we don't care about it if it // is not still valid bool remove_reference = false; - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(true/*waiter*/); if (has_reference) { AutoLock n_lock(node_lock); @@ -8607,7 +8712,7 @@ namespace Legion { assert(record.node == this); #endif bool remove_reference = false; - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(true/*waiter*/); { AutoLock n_lock(node_lock); { @@ -8657,7 +8762,6 @@ namespace Legion { //-------------------------------------------------------------------------- { bool remove_reference; - LocalReferenceMutator mutator; { AutoLock n_lock(node_lock); remove_reference = (--send_references == 0); @@ -8994,7 +9098,8 @@ namespace Legion { } //-------------------------------------------------------------------------- - void IndexSpaceNode::pack_expression(Serializer &rez, AddressSpaceID target) + void IndexSpaceNode::pack_expression(Serializer &rez, AddressSpaceID target, + bool need_reference) //-------------------------------------------------------------------------- { if (target != context->runtime->address_space) @@ -9006,7 +9111,10 @@ namespace Legion { else { rez.serialize(true/*local*/); + rez.serialize(need_reference); rez.serialize(this); + if (need_reference) + add_base_expression_reference(LIVE_EXPR_REF); } } @@ -9021,7 +9129,7 @@ namespace Legion { // This could be a performance bug since it will block if we // have to send a reference to a remote node, but that should // never actually happen - LocalReferenceMutator mutator; + LocalReferenceMutator mutator(true/*waiter*/); add_base_gc_ref(REMOTE_DID_REF, &mutator); } @@ -9060,7 +9168,7 @@ namespace Legion { { if (mutator == NULL) { - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(true/*waiter*/); add_base_valid_ref(source, &local_mutator, count); } else @@ -9083,7 +9191,7 @@ namespace Legion { { if (mutator == NULL) { - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(true/*waiter*/); add_nested_valid_ref(source, &local_mutator, count); } else diff --git a/runtime/legion/region_tree.h b/runtime/legion/region_tree.h index ddd6e93b3b..af32f82af3 100644 --- a/runtime/legion/region_tree.h +++ b/runtime/legion/region_tree.h @@ -43,7 +43,7 @@ namespace Legion { /** * \struct IndirectRecord - * A small helper calss for performing exchanges of + * A small helper class for performing exchanges of * instances for indirection copies */ struct IndirectRecord : public LegionHeapify { @@ -62,6 +62,24 @@ namespace Legion { Domain domain; }; + /** + * \struct PendingRemoteExpression + * A small helper class for passing arguments associated + * with deferred calls to unpack remote expressions + */ + struct PendingRemoteExpression { + public: + PendingRemoteExpression(void) + : handle(IndexSpace::NO_SPACE), remote_expr_id(0), + source(0), is_index_space(false), has_reference(false) { } + public: + IndexSpace handle; + IndexSpaceExprID remote_expr_id; + AddressSpaceID source; + bool is_index_space; + bool has_reference; + }; + /** * \class OperationCreator * A base class for handling the creation of index space operations @@ -877,7 +895,7 @@ namespace Legion { IndexSpaceExprID remote_expr_id, IndexSpaceExpression *origin, RtEvent *wait_for = NULL); IndexSpaceExpression* find_remote_expression( - IndexSpaceExprID remote_expr_id); + const PendingRemoteExpression &pending_expression); void unregister_remote_expression(IndexSpaceExprID remote_expr_id); void handle_remote_expression_request(Deserializer &derez, AddressSpaceID source); @@ -1110,7 +1128,8 @@ namespace Legion { virtual void tighten_index_space(void) = 0; virtual bool check_empty(void) = 0; virtual size_t get_volume(void) = 0; - virtual void pack_expression(Serializer &rez, AddressSpaceID target) = 0; + virtual void pack_expression(Serializer &rez, AddressSpaceID target, + bool need_reference = true) = 0; virtual void pack_expression_value(Serializer &rez, AddressSpaceID target) = 0; public: @@ -1328,11 +1347,10 @@ namespace Legion { std::set &expressions); public: static IndexSpaceExpression* unpack_expression(Deserializer &derez, - RegionTreeForest *forest, AddressSpaceID source); + RegionTreeForest *forest, AddressSpaceID source); static IndexSpaceExpression* unpack_expression(Deserializer &derez, RegionTreeForest *forest, AddressSpaceID source, - bool &is_local,bool &is_index_space,IndexSpace &handle, - IndexSpaceExprID &remote_expr_id, RtEvent &wait_for); + PendingRemoteExpression &pending, RtEvent &wait_for); public: const TypeTag type_tag; const IndexSpaceExprID expr_id; @@ -1360,7 +1378,7 @@ namespace Legion { { if (m == NULL) { - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(true/*waiting*/); expr->add_base_expression_reference(LIVE_EXPR_REF, &local_mutator); } else @@ -1452,7 +1470,8 @@ namespace Legion { virtual void tighten_index_space(void) = 0; virtual bool check_empty(void) = 0; virtual size_t get_volume(void) = 0; - virtual void pack_expression(Serializer &rez, AddressSpaceID target) = 0; + virtual void pack_expression(Serializer &rez, AddressSpaceID target, + bool need_reference = true) = 0; virtual void pack_expression_value(Serializer &rez, AddressSpaceID target) = 0; public: @@ -1512,7 +1531,8 @@ namespace Legion { virtual void tighten_index_space(void); virtual bool check_empty(void); virtual size_t get_volume(void); - virtual void pack_expression(Serializer &rez, AddressSpaceID target); + virtual void pack_expression(Serializer &rez, AddressSpaceID target, + bool need_reference = true); virtual void pack_expression_value(Serializer &rez, AddressSpaceID target) = 0; virtual bool invalidate_operation(void) = 0; @@ -2080,7 +2100,8 @@ namespace Legion { virtual bool set_domain(const Domain &domain, AddressSpaceID space) = 0; virtual void tighten_index_space(void) = 0; virtual bool check_empty(void) = 0; - virtual void pack_expression(Serializer &rez, AddressSpaceID target); + virtual void pack_expression(Serializer &rez, AddressSpaceID target, + bool need_reference); virtual void pack_expression_value(Serializer &rez,AddressSpaceID target); public: #ifdef DEBUG_LEGION diff --git a/runtime/legion/region_tree.inl b/runtime/legion/region_tree.inl index 7c5ed0420c..d0c57c4c7f 100644 --- a/runtime/legion/region_tree.inl +++ b/runtime/legion/region_tree.inl @@ -1534,25 +1534,42 @@ namespace Legion { //-------------------------------------------------------------------------- template void IndexSpaceOperationT::pack_expression(Serializer &rez, - AddressSpaceID target) + AddressSpaceID target, bool need_reference) //-------------------------------------------------------------------------- { +#ifdef DEBUG_LEGION + assert(this->is_valid()); +#endif if (target == this->local_space) { rez.serialize(true/*local*/); + rez.serialize(need_reference); rez.serialize(this); + // Add a live expression reference to keep this live through the message + if (need_reference) + this->add_base_expression_reference(LIVE_EXPR_REF); } else if (target == this->owner_space) { rez.serialize(true/*local*/); + rez.serialize(need_reference); rez.serialize(origin_expr); + // Add a reference here that we'll remove after we've added a reference + // on the target space expression + if (need_reference) + this->add_base_expression_reference(REMOTE_DID_REF); } else { rez.serialize(false/*local*/); rez.serialize(false/*index space*/); + rez.serialize(need_reference); rez.serialize(expr_id); rez.serialize(origin_expr); + // Add a reference here that we'll remove after we've added a reference + // on the target space expression + if (need_reference) + this->add_base_expression_reference(REMOTE_DID_REF); } } @@ -2339,9 +2356,9 @@ namespace Legion { { // This is another kind of live expression made by the region tree this->add_base_expression_reference(LIVE_EXPR_REF); - if (implicit_live_expressions == NULL) - implicit_live_expressions = new std::vector; - implicit_live_expressions->emplace_back(this); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(this); #ifdef DEBUG_LEGION assert(num_rects > 0); #endif @@ -2534,8 +2551,7 @@ namespace Legion { void RemoteExpression::remove_operation(void) //-------------------------------------------------------------------------- { - // should never be called - assert(false); + // nothing to do here } ///////////////////////////////////////////////////////////// diff --git a/runtime/legion/runtime.cc b/runtime/legion/runtime.cc index 978b1c2706..18ba843816 100644 --- a/runtime/legion/runtime.cc +++ b/runtime/legion/runtime.cc @@ -75,7 +75,7 @@ namespace Legion { __thread AutoLock *local_lock_list = NULL; __thread UniqueID implicit_provenance = 0; __thread unsigned inside_registration_callback = NO_REGISTRATION_CALLBACK; - __thread std::vector *implicit_live_expressions=NULL; + __thread ImplicitReferenceTracker *implicit_reference_tracker = NULL; const LgEvent LgEvent::NO_LG_EVENT = LgEvent(); const ApEvent ApEvent::NO_AP_EVENT = ApEvent(); @@ -5176,7 +5176,6 @@ namespace Legion { //-------------------------------------------------------------------------- { bool remove_min_reference = false; - IgnoreReferenceMutator mutator; if (!is_owner) { RtUserEvent never_gc_wait; @@ -5255,14 +5254,14 @@ namespace Legion { bool remove_duplicate = false; if (success.load()) { - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(true/*waiter*/); // Add our local reference manager->add_base_valid_ref(NEVER_GC_REF, &local_mutator); const RtEvent reference_effects = local_mutator.get_done_event(); manager->send_remote_valid_decrement(owner_space, NULL, reference_effects); if (reference_effects.exists()) - mutator.record_reference_mutation_effect(reference_effects); + local_mutator.record_reference_mutation_effect(reference_effects); // Then record it AutoLock m_lock(manager_lock); #ifdef DEBUG_LEGION @@ -5279,7 +5278,7 @@ namespace Legion { info.mapper_priorities[key] = LEGION_GC_NEVER_PRIORITY; } if (remove_duplicate && - manager->remove_base_valid_ref(NEVER_GC_REF, &mutator)) + manager->remove_base_valid_ref(NEVER_GC_REF)) delete manager; } } @@ -5288,7 +5287,7 @@ namespace Legion { // If this a max priority, try adding the reference beforehand, if // it fails then we know the instance is already deleted so whatever if ((priority == LEGION_GC_NEVER_PRIORITY) && - !manager->acquire_instance(NEVER_GC_REF, &mutator)) + !manager->acquire_instance(NEVER_GC_REF, NULL/*mutator*/)) return; // Do the update locally AutoLock m_lock(manager_lock); @@ -5363,8 +5362,7 @@ namespace Legion { } } } - if (remove_min_reference && - manager->remove_base_valid_ref(NEVER_GC_REF, &mutator)) + if (remove_min_reference && manager->remove_base_valid_ref(NEVER_GC_REF)) delete manager; } @@ -5971,7 +5969,7 @@ namespace Legion { // and then remove the remote DID if (acquire) { - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(false/*waiter*/); manager->add_base_valid_ref(MAPPING_ACQUIRE_REF, &local_mutator); const RtEvent reference_effects = local_mutator.get_done_event(); manager->send_remote_valid_decrement(source, NULL, @@ -6105,7 +6103,7 @@ namespace Legion { // and then remove the remote DID if (acquire) { - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(true/*waiter*/); manager->add_base_valid_ref(MAPPING_ACQUIRE_REF, &local_mutator); const RtEvent reference_effects = local_mutator.get_done_event(); manager->send_remote_valid_decrement(source, NULL, @@ -6325,7 +6323,7 @@ namespace Legion { (*target)[index] = true; PhysicalManager *manager; derez.deserialize(manager); - LocalReferenceMutator local_mutator; + LocalReferenceMutator local_mutator(false/*waiter*/); manager->add_base_valid_ref(MAPPING_ACQUIRE_REF, &local_mutator); const RtEvent reference_effects = local_mutator.get_done_event(); manager->send_remote_valid_decrement(source, NULL, reference_effects); @@ -13145,23 +13143,17 @@ namespace Legion { //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION - assert(implicit_live_expressions == NULL); + assert(implicit_reference_tracker == NULL); #endif if (ctx != DUMMY_CONTEXT) ctx->begin_runtime_call(); const bool result = forest->is_index_partition_disjoint(p); if (ctx != DUMMY_CONTEXT) ctx->end_runtime_call(); - else if (implicit_live_expressions != NULL) + else if (implicit_reference_tracker != NULL) { - // Remove references to any live index space expressions we have - for (std::vector::const_iterator it = - implicit_live_expressions->begin(); it != - implicit_live_expressions->end(); it++) - if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) - delete (*it); - delete implicit_live_expressions; - implicit_live_expressions = NULL; + delete implicit_reference_tracker; + implicit_reference_tracker = NULL; } return result; } @@ -13171,19 +13163,13 @@ namespace Legion { //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION - assert(implicit_live_expressions == NULL); + assert(implicit_reference_tracker == NULL); #endif const bool result = forest->is_index_partition_disjoint(p); - if (implicit_live_expressions != NULL) + if (implicit_reference_tracker != NULL) { - // Remove references to any live index space expressions we have - for (std::vector::const_iterator it = - implicit_live_expressions->begin(); it != - implicit_live_expressions->end(); it++) - if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) - delete (*it); - delete implicit_live_expressions; - implicit_live_expressions = NULL; + delete implicit_reference_tracker; + implicit_reference_tracker = NULL; } return result; } @@ -13193,23 +13179,17 @@ namespace Legion { //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION - assert(implicit_live_expressions == NULL); + assert(implicit_reference_tracker == NULL); #endif if (ctx != DUMMY_CONTEXT) ctx->begin_runtime_call(); bool result = forest->is_index_partition_complete(p); if (ctx != DUMMY_CONTEXT) ctx->end_runtime_call(); - else if (implicit_live_expressions != NULL) + else if (implicit_reference_tracker != NULL) { - // Remove references to any live index space expressions we have - for (std::vector::const_iterator it = - implicit_live_expressions->begin(); it != - implicit_live_expressions->end(); it++) - if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) - delete (*it); - delete implicit_live_expressions; - implicit_live_expressions = NULL; + delete implicit_reference_tracker; + implicit_reference_tracker = NULL; } return result; } @@ -13219,19 +13199,13 @@ namespace Legion { //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION - assert(implicit_live_expressions == NULL); + assert(implicit_reference_tracker == NULL); #endif const bool result = forest->is_index_partition_complete(p); - if (implicit_live_expressions != NULL) + if (implicit_reference_tracker != NULL) { - // Remove references to any live index space expressions we have - for (std::vector::const_iterator it = - implicit_live_expressions->begin(); it != - implicit_live_expressions->end(); it++) - if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) - delete (*it); - delete implicit_live_expressions; - implicit_live_expressions = NULL; + delete implicit_reference_tracker; + implicit_reference_tracker = NULL; } return result; } @@ -24429,7 +24403,7 @@ namespace Legion { if (!runtime->local_utils.empty()) assert(implicit_context == NULL); // this better hold #endif - assert(implicit_live_expressions == NULL); + assert(implicit_reference_tracker == NULL); #endif implicit_runtime = runtime; // We immediately bump the priority of all meta-tasks once they start @@ -25060,16 +25034,10 @@ namespace Legion { default: assert(false); // should never get here } - if (implicit_live_expressions != NULL) + if (implicit_reference_tracker != NULL) { - // Remove references to any live index space expressions we have - for (std::vector::const_iterator it = - implicit_live_expressions->begin(); it != - implicit_live_expressions->end(); it++) - if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) - delete (*it); - delete implicit_live_expressions; - implicit_live_expressions = NULL; + delete implicit_reference_tracker; + implicit_reference_tracker = NULL; } #ifdef DEBUG_LEGION if (tid < LG_BEGIN_SHUTDOWN_TASK_IDS) @@ -25148,7 +25116,7 @@ namespace Legion { Runtime *runtime = *((Runtime**)userdata); #ifdef DEBUG_LEGION assert(userlen == sizeof(Runtime**)); - assert(implicit_live_expressions == NULL); + assert(implicit_reference_tracker == NULL); #endif implicit_runtime = runtime; // We immediately bump the priority of all meta-tasks once they start @@ -25188,16 +25156,10 @@ namespace Legion { default: assert(false); // should never get here } - if (implicit_live_expressions != NULL) + if (implicit_reference_tracker != NULL) { - // Remove references to any live index space expressions we have - for (std::vector::const_iterator it = - implicit_live_expressions->begin(); it != - implicit_live_expressions->end(); it++) - if ((*it)->remove_base_expression_reference(LIVE_EXPR_REF)) - delete (*it); - delete implicit_live_expressions; - implicit_live_expressions = NULL; + delete implicit_reference_tracker; + implicit_reference_tracker = NULL; } #ifdef DEBUG_LEGION runtime->decrement_total_outstanding_tasks(tid, true/*meta*/); From 9cb66ba80bba622ccaab950e6bcffcdfc32caeba Mon Sep 17 00:00:00 2001 From: Mike Bauer Date: Mon, 13 Dec 2021 19:49:51 -0800 Subject: [PATCH 7/9] legion: more work on distributed reference counting for expressions --- runtime/legion/legion_analysis.cc | 9 +- runtime/legion/legion_analysis.h | 3 +- runtime/legion/legion_instances.cc | 4 +- runtime/legion/legion_types.h | 4 - runtime/legion/legion_views.cc | 11 +- runtime/legion/region_tree.cc | 264 +++++++++++++++++++---------- runtime/legion/region_tree.h | 18 +- runtime/legion/region_tree.inl | 15 +- 8 files changed, 192 insertions(+), 136 deletions(-) diff --git a/runtime/legion/legion_analysis.cc b/runtime/legion/legion_analysis.cc index a929dbae3f..ac0b07211b 100644 --- a/runtime/legion/legion_analysis.cc +++ b/runtime/legion/legion_analysis.cc @@ -130,8 +130,8 @@ namespace Legion { } //-------------------------------------------------------------------------- - void PhysicalUser::pack_user(Serializer &rez, const AddressSpaceID target, - bool need_reference) const + void PhysicalUser::pack_user(Serializer &rez, + const AddressSpaceID target) const //-------------------------------------------------------------------------- { RezCheck z(rez); @@ -139,7 +139,7 @@ namespace Legion { rez.serialize(collect_event); #endif rez.serialize(usage); - expr->pack_expression(rez, target, need_reference); + expr->pack_expression(rez, target); rez.serialize(op_id); rez.serialize(index); rez.serialize(copy_user); @@ -13558,8 +13558,7 @@ namespace Legion { else rez.serialize(logical_owner_space); } - // No need for a reference here since we know we'll continue holding it - set_expr->pack_expression(rez, target, false/*need reference*/); + set_expr->pack_expression(rez, target); if (index_space_node != NULL) rez.serialize(index_space_node->handle); else diff --git a/runtime/legion/legion_analysis.h b/runtime/legion/legion_analysis.h index 116d605d78..26015fd7cf 100644 --- a/runtime/legion/legion_analysis.h +++ b/runtime/legion/legion_analysis.h @@ -705,8 +705,7 @@ namespace Legion { public: PhysicalUser& operator=(const PhysicalUser &rhs); public: - void pack_user(Serializer &rez, const AddressSpaceID target, - bool need_reference = true) const; + void pack_user(Serializer &rez, const AddressSpaceID target) const; static PhysicalUser* unpack_user(Deserializer &derez, RegionTreeForest *forest, const AddressSpaceID source); public: diff --git a/runtime/legion/legion_instances.cc b/runtime/legion/legion_instances.cc index 83be9c6656..f2dffa957d 100644 --- a/runtime/legion/legion_instances.cc +++ b/runtime/legion/legion_instances.cc @@ -1480,7 +1480,7 @@ namespace Legion { rez.serialize(instance); rez.serialize(instance_footprint); // No need for a reference here since we know we'll continue holding it - instance_domain->pack_expression(rez, target, false/*need reference*/); + instance_domain->pack_expression(rez, target); rez.serialize(piece_list_size); if (piece_list_size > 0) rez.serialize(piece_list, piece_list_size); @@ -2902,7 +2902,7 @@ namespace Legion { rez.serialize(point_space->handle); rez.serialize(instance_footprint); // No need for a reference here since we know we'll continue holding it - instance_domain->pack_expression(rez, target, false/*need reference*/); + instance_domain->pack_expression(rez, target); rez.serialize(field_space_node->handle); rez.serialize(tree_id); rez.serialize(redop); diff --git a/runtime/legion/legion_types.h b/runtime/legion/legion_types.h index cdfc372de4..1eff2f0a1a 100644 --- a/runtime/legion/legion_types.h +++ b/runtime/legion/legion_types.h @@ -2388,9 +2388,7 @@ namespace Legion { unsigned local_callback = Internal::inside_registration_callback; // Save the reference tracker that we have ImplicitReferenceTracker *local_tracker = implicit_reference_tracker; -#ifdef DEBUG_LEGION Internal::implicit_reference_tracker = NULL; -#endif // Check to see if we have any local locks to notify if (Internal::local_lock_list != NULL) { @@ -2448,9 +2446,7 @@ namespace Legion { unsigned local_callback = Internal::inside_registration_callback; // Save the reference tracker that we have ImplicitReferenceTracker *local_tracker = implicit_reference_tracker; -#ifdef DEBUG_LEGION Internal::implicit_reference_tracker = NULL; -#endif // Check to see if we have any local locks to notify if (Internal::local_lock_list != NULL) { diff --git a/runtime/legion/legion_views.cc b/runtime/legion/legion_views.cc index 4a6face762..b1750141d7 100644 --- a/runtime/legion/legion_views.cc +++ b/runtime/legion/legion_views.cc @@ -1580,9 +1580,7 @@ namespace Legion { { const unsigned index = indexes.size(); rez.serialize(index); - // No need for a reference since we're replicating so we know - // that we'll continue holding this reference - it->first->pack_user(rez, target, false/*need reference*/); + it->first->pack_user(rez, target); indexes[it->first] = index; } else @@ -1623,9 +1621,7 @@ namespace Legion { { const unsigned index = indexes.size(); rez.serialize(index); - // No need for a reference since we're replicating so we know - // that we'll continue holding the reference to keep it alive - it->first->pack_user(rez, target, false/*need reference*/); + it->first->pack_user(rez, target); indexes[it->first] = index; } else @@ -1650,8 +1646,7 @@ namespace Legion { for (FieldMaskSet::const_iterator it = needed_subviews.begin(); it != needed_subviews.end(); it++) { - // No need for the reference since we're replicating this - it->first->view_expr->pack_expression(rez, target, false/*need ref*/); + it->first->view_expr->pack_expression(rez, target); rez.serialize(it->second); it->first->pack_replication(rez, indexes, it->second, target); } diff --git a/runtime/legion/region_tree.cc b/runtime/legion/region_tree.cc index a8d096ae74..493fcda22d 100644 --- a/runtime/legion/region_tree.cc +++ b/runtime/legion/region_tree.cc @@ -6571,19 +6571,38 @@ namespace Legion { //-------------------------------------------------------------------------- { if (pending.is_index_space) - return get_node(pending.handle); - IndexSpaceExpression *result = NULL; { - AutoLock l_lock(lookup_is_op_lock, 1, false/*exclusive*/); - std::map::const_iterator - finder = remote_expressions.find(pending.remote_expr_id); -#ifdef DEBUG_LEGION - assert(finder != remote_expressions.end()); -#endif - result = finder->second; + IndexSpaceNode *node = get_node(pending.handle); + LocalReferenceMutator mutator(false/*waiter*/); + node->add_base_expression_reference(LIVE_EXPR_REF, &mutator); + const RtEvent added = mutator.get_done_event(); + // Special case here: if the source was the owner and we didn't + // just send a message to add our reference then we can buffer up + // our reference to be removed when we are no longer valid + // Be very careful here! You can only do this if the expression + // was sent from the source or you risk reference count cycles! + if ((pending.source == node->owner_space) && + (!added.exists() || added.has_triggered())) + node->record_remote_owner_valid_reference(); + else + node->send_remote_valid_decrement(pending.source, NULL/*mut*/, added); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(node); + return node; } - if (pending.has_reference) + else { + IndexSpaceExpression *result = NULL; + { + AutoLock l_lock(lookup_is_op_lock, 1, false/*exclusive*/); + std::map::const_iterator + finder = remote_expressions.find(pending.remote_expr_id); +#ifdef DEBUG_LEGION + assert(finder != remote_expressions.end()); +#endif + result = finder->second; + } #ifdef DEBUG_LEGION IndexSpaceOperation *op = dynamic_cast(result); assert(op != NULL); @@ -6592,13 +6611,22 @@ namespace Legion { #endif LocalReferenceMutator mutator(false/*waiter*/); result->add_base_expression_reference(LIVE_EXPR_REF, &mutator); - op->send_remote_valid_decrement(pending.source, NULL/*mutator*/, - mutator.get_done_event()); + const RtEvent added = mutator.get_done_event(); + // Special case here: if the source was the owner and we didn't + // just send a message to add our reference then we can buffer up + // our reference to be removed when we are no longer valid + // Be very careful here! You can only do this if the expression + // was sent from the source or you risk reference count cycles! + if ((pending.source == op->owner_space) && + (!added.exists() || added.has_triggered())) + result->record_remote_owner_valid_reference(); + else + op->send_remote_valid_decrement(pending.source,NULL/*mutator*/,added); if (implicit_reference_tracker == NULL) implicit_reference_tracker = new ImplicitReferenceTracker; implicit_reference_tracker->record_live_expression(result); + return result; } - return result; } //-------------------------------------------------------------------------- @@ -6693,8 +6721,9 @@ namespace Legion { //-------------------------------------------------------------------------- IndexSpaceExpression::IndexSpaceExpression(LocalLock &lock) - : type_tag(0), expr_id(0), expr_lock(lock), canonical(NULL), volume(0), - has_volume(false), empty(false), has_empty(false) + : type_tag(0), expr_id(0), expr_lock(lock), canonical(NULL), + remote_owner_valid_references(0), + volume(0), has_volume(false), empty(false), has_empty(false) //-------------------------------------------------------------------------- { } @@ -6703,8 +6732,8 @@ namespace Legion { IndexSpaceExpression::IndexSpaceExpression(TypeTag tag, Runtime *rt, LocalLock &lock) : type_tag(tag), expr_id(rt->get_unique_index_space_expr_id()), - expr_lock(lock), canonical(NULL), volume(0), has_volume(false), - empty(false), has_empty(false) + expr_lock(lock), canonical(NULL), remote_owner_valid_references(0), + volume(0), has_volume(false), empty(false), has_empty(false) //-------------------------------------------------------------------------- { } @@ -6712,8 +6741,9 @@ namespace Legion { //-------------------------------------------------------------------------- IndexSpaceExpression::IndexSpaceExpression(TypeTag tag, IndexSpaceExprID id, LocalLock &lock) - : type_tag(tag), expr_id(id), expr_lock(lock), canonical(NULL), volume(0), - has_volume(false), empty(false), has_empty(false) + : type_tag(tag), expr_id(id), expr_lock(lock), canonical(NULL), + remote_owner_valid_references(0), + volume(0), has_volume(false), empty(false), has_empty(false) //-------------------------------------------------------------------------- { } @@ -6837,32 +6867,29 @@ namespace Legion { derez.deserialize(is_local); if (is_local) { - bool has_reference; - derez.deserialize(has_reference); IndexSpaceExpression *result; derez.deserialize(result); - if (has_reference) + if (source != forest->runtime->address_space) { - if (source != forest->runtime->address_space) - { #ifdef DEBUG_LEGION - IndexSpaceOperation *op = - dynamic_cast(result); - assert(op != NULL); + IndexSpaceOperation *op = + dynamic_cast(result); + assert(op != NULL); #else - IndexSpaceOperation *op = static_cast(result); -#endif - // Make this valid and then send the removal of the - // remote did expression - LocalReferenceMutator mutator(false/*waiter*/); - op->add_base_expression_reference(LIVE_EXPR_REF, &mutator); - op->send_remote_valid_decrement(source, NULL/*mutator*/, - mutator.get_done_event()); - } - if (implicit_reference_tracker == NULL) - implicit_reference_tracker = new ImplicitReferenceTracker; - implicit_reference_tracker->record_live_expression(result); + IndexSpaceOperation *op = static_cast(result); +#endif + // Make this valid and then send the removal of the + // remote did expression + LocalReferenceMutator mutator(false/*waiter*/); + op->add_base_expression_reference(LIVE_EXPR_REF, &mutator); + // Always need to send this reference removal back immediately + // in order to avoid reference counting deadlock + op->send_remote_valid_decrement(source, NULL/*mutator*/, + mutator.get_done_event()); } + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); return result; } bool is_index_space; @@ -6872,18 +6899,33 @@ namespace Legion { { IndexSpace handle; derez.deserialize(handle); - return forest->get_node(handle); + IndexSpaceNode *node = forest->get_node(handle); + LocalReferenceMutator mutator(false/*waiter*/); + node->add_base_expression_reference(LIVE_EXPR_REF, &mutator); + const RtEvent added = mutator.get_done_event(); + // Special case here: if the source was the owner and we didn't + // just send a message to add our reference then we can buffer up + // our reference to be removed when we are no longer valid + // Be very careful here! You can only do this if the expression + // was sent from the source or you risk reference count cycles! + if ((source == node->owner_space) && + (!added.exists() || added.has_triggered())) + node->record_remote_owner_valid_reference(); + else + node->send_remote_valid_decrement(source, NULL/*mutator*/, added); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(node); + return node; } - bool has_reference; - derez.deserialize(has_reference); - IndexSpaceExprID remote_expr_id; - derez.deserialize(remote_expr_id); - IndexSpaceExpression *origin; - derez.deserialize(origin); - IndexSpaceExpression *result = - forest->find_or_request_remote_expression(remote_expr_id, origin); - if (has_reference) + else { + IndexSpaceExprID remote_expr_id; + derez.deserialize(remote_expr_id); + IndexSpaceExpression *origin; + derez.deserialize(origin); + IndexSpaceExpression *result = + forest->find_or_request_remote_expression(remote_expr_id, origin); #ifdef DEBUG_LEGION IndexSpaceOperation *op = dynamic_cast(result); assert(op != NULL); @@ -6892,13 +6934,22 @@ namespace Legion { #endif LocalReferenceMutator mutator(false/*waiter*/); result->add_base_expression_reference(LIVE_EXPR_REF, &mutator); - op->send_remote_valid_decrement(source, NULL/*mutator*/, - mutator.get_done_event()); + const RtEvent added = mutator.get_done_event(); + // Special case here: if the source was the owner and we didn't + // just send a message to add our reference then we can buffer up + // our reference to be removed when we are no longer valid + // Be very careful here! You can only do this if the expression + // was sent from the source or you risk reference count cycles! + if ((source == op->owner_space) && + (!added.exists() || added.has_triggered())) + result->record_remote_owner_valid_reference(); + else + op->send_remote_valid_decrement(source, NULL/*mutator*/, added); if (implicit_reference_tracker == NULL) implicit_reference_tracker = new ImplicitReferenceTracker; implicit_reference_tracker->record_live_expression(result); + return result; } - return result; } //-------------------------------------------------------------------------- @@ -6912,31 +6963,29 @@ namespace Legion { derez.deserialize(is_local); if (is_local) { - derez.deserialize(pending.has_reference); IndexSpaceExpression *result; derez.deserialize(result); - if (pending.has_reference) + if (source != forest->runtime->address_space) { - if (source != forest->runtime->address_space) - { #ifdef DEBUG_LEGION - IndexSpaceOperation *op = - dynamic_cast(result); - assert(op != NULL); + IndexSpaceOperation *op = + dynamic_cast(result); + assert(op != NULL); #else - IndexSpaceOperation *op = static_cast(result); -#endif - // Make this valid and then send the removal of the - // remote did expression - LocalReferenceMutator mutator(false/*waiter*/); - op->add_base_expression_reference(LIVE_EXPR_REF, &mutator); - op->send_remote_valid_decrement(source, NULL/*mutator*/, - mutator.get_done_event()); - } - if (implicit_reference_tracker == NULL) - implicit_reference_tracker = new ImplicitReferenceTracker; - implicit_reference_tracker->record_live_expression(result); + IndexSpaceOperation *op = static_cast(result); +#endif + // Make this valid and then send the removal of the + // remote did expression + LocalReferenceMutator mutator(false/*waiter*/); + op->add_base_expression_reference(LIVE_EXPR_REF, &mutator); + // Always need to send this reference removal back immediately + // in order to avoid reference counting deadlock + op->send_remote_valid_decrement(source, NULL/*mutator*/, + mutator.get_done_event()); } + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); return result; } derez.deserialize(pending.is_index_space); @@ -6944,9 +6993,30 @@ namespace Legion { if (pending.is_index_space) { derez.deserialize(pending.handle); - return forest->get_node(pending.handle, &wait_for); + IndexSpaceNode *node = forest->get_node(pending.handle, &wait_for); + if (node == NULL) + { + pending.source = source; + return node; + } + LocalReferenceMutator mutator(false/*waiter*/); + node->add_base_expression_reference(LIVE_EXPR_REF, &mutator); + const RtEvent added = mutator.get_done_event(); + // Special case here: if the source was the owner and we didn't + // just send a message to add our reference then we can buffer up + // our reference to be removed when we are no longer valid + // Be very careful here! You can only do this if the expression + // was sent from the source or you risk reference count cycles! + if ((source == node->owner_space) && + (!added.exists() || added.has_triggered())) + node->record_remote_owner_valid_reference(); + else + node->send_remote_valid_decrement(source, NULL/*mutator*/, added); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(node); + return node; } - derez.deserialize(pending.has_reference); derez.deserialize(pending.remote_expr_id); IndexSpaceExpression *origin; derez.deserialize(origin); @@ -6958,22 +7028,28 @@ namespace Legion { pending.source = source; return result; } - if (pending.has_reference) - { #ifdef DEBUG_LEGION - IndexSpaceOperation *op = dynamic_cast(result); - assert(op != NULL); + IndexSpaceOperation *op = dynamic_cast(result); + assert(op != NULL); #else - IndexSpaceOperation *op = static_cast(result); -#endif - LocalReferenceMutator mutator(false/*waiter*/); - result->add_base_expression_reference(LIVE_EXPR_REF, &mutator); - op->send_remote_valid_decrement(source, NULL/*mutator*/, - mutator.get_done_event()); - if (implicit_reference_tracker == NULL) - implicit_reference_tracker = new ImplicitReferenceTracker; - implicit_reference_tracker->record_live_expression(result); - } + IndexSpaceOperation *op = static_cast(result); +#endif + LocalReferenceMutator mutator(false/*waiter*/); + result->add_base_expression_reference(LIVE_EXPR_REF, &mutator); + const RtEvent added = mutator.get_done_event(); + // Special case here: if the source was the owner and we didn't + // just send a message to add our reference then we can buffer up + // our reference to be removed when we are no longer valid + // Be very careful here! You can only do this if the expression + // was sent from the source or you risk reference count cycles! + if ((source == op->owner_space) && + (!added.exists() || added.has_triggered())) + result->record_remote_owner_valid_reference(); + else + op->send_remote_valid_decrement(source, NULL/*mutator*/, added); + if (implicit_reference_tracker == NULL) + implicit_reference_tracker = new ImplicitReferenceTracker; + implicit_reference_tracker->record_live_expression(result); return result; } @@ -7064,7 +7140,8 @@ namespace Legion { //-------------------------------------------------------------------------- { if (!is_owner()) - send_remote_valid_decrement(owner_space, mutator); + send_remote_valid_decrement(owner_space, mutator, + RtEvent::NO_RT_EVENT, remote_owner_valid_references.exchange(0) + 1); // If we have a canonical reference that is not ourselves then // we need to remove the nested reference that we are holding on it too if ((canonical != NULL) && (canonical != this) && @@ -7938,7 +8015,8 @@ namespace Legion { tree_valid = false; } else - send_remote_valid_decrement(owner_space, mutator); + send_remote_valid_decrement(owner_space, mutator, + RtEvent::NO_RT_EVENT, remote_owner_valid_references.exchange(0) + 1); // If we have a canonical reference that is not ourselves then // we need to remove the nested reference that we are holding on it too if ((canonical != NULL) && (canonical != this) && @@ -9098,8 +9176,7 @@ namespace Legion { } //-------------------------------------------------------------------------- - void IndexSpaceNode::pack_expression(Serializer &rez, AddressSpaceID target, - bool need_reference) + void IndexSpaceNode::pack_expression(Serializer &rez, AddressSpaceID target) //-------------------------------------------------------------------------- { if (target != context->runtime->address_space) @@ -9107,14 +9184,13 @@ namespace Legion { rez.serialize(false/*local*/); rez.serialize(true/*index space*/); rez.serialize(handle); + add_base_expression_reference(REMOTE_DID_REF); } else { rez.serialize(true/*local*/); - rez.serialize(need_reference); rez.serialize(this); - if (need_reference) - add_base_expression_reference(LIVE_EXPR_REF); + add_base_expression_reference(LIVE_EXPR_REF); } } diff --git a/runtime/legion/region_tree.h b/runtime/legion/region_tree.h index af32f82af3..261ae001e7 100644 --- a/runtime/legion/region_tree.h +++ b/runtime/legion/region_tree.h @@ -71,13 +71,12 @@ namespace Legion { public: PendingRemoteExpression(void) : handle(IndexSpace::NO_SPACE), remote_expr_id(0), - source(0), is_index_space(false), has_reference(false) { } + source(0), is_index_space(false) { } public: IndexSpace handle; IndexSpaceExprID remote_expr_id; AddressSpaceID source; bool is_index_space; - bool has_reference; }; /** @@ -1128,8 +1127,7 @@ namespace Legion { virtual void tighten_index_space(void) = 0; virtual bool check_empty(void) = 0; virtual size_t get_volume(void) = 0; - virtual void pack_expression(Serializer &rez, AddressSpaceID target, - bool need_reference = true) = 0; + virtual void pack_expression(Serializer &rez, AddressSpaceID target) = 0; virtual void pack_expression_value(Serializer &rez, AddressSpaceID target) = 0; public: @@ -1251,6 +1249,8 @@ namespace Legion { } inline size_t get_num_dims(void) const { return NT_TemplateHelper::get_dim(type_tag); } + inline void record_remote_owner_valid_reference(void) + { remote_owner_valid_references.fetch_add(1); } public: // Convert this index space expression to the canonical one that // represents all expressions that are all congruent @@ -1359,6 +1359,7 @@ namespace Legion { protected: std::set derived_operations; IndexSpaceExpression *canonical; + std::atomic remote_owner_valid_references; size_t volume; bool has_volume; bool empty, has_empty; @@ -1470,8 +1471,7 @@ namespace Legion { virtual void tighten_index_space(void) = 0; virtual bool check_empty(void) = 0; virtual size_t get_volume(void) = 0; - virtual void pack_expression(Serializer &rez, AddressSpaceID target, - bool need_reference = true) = 0; + virtual void pack_expression(Serializer &rez, AddressSpaceID target) = 0; virtual void pack_expression_value(Serializer &rez, AddressSpaceID target) = 0; public: @@ -1531,8 +1531,7 @@ namespace Legion { virtual void tighten_index_space(void); virtual bool check_empty(void); virtual size_t get_volume(void); - virtual void pack_expression(Serializer &rez, AddressSpaceID target, - bool need_reference = true); + virtual void pack_expression(Serializer &rez, AddressSpaceID target); virtual void pack_expression_value(Serializer &rez, AddressSpaceID target) = 0; virtual bool invalidate_operation(void) = 0; @@ -2100,8 +2099,7 @@ namespace Legion { virtual bool set_domain(const Domain &domain, AddressSpaceID space) = 0; virtual void tighten_index_space(void) = 0; virtual bool check_empty(void) = 0; - virtual void pack_expression(Serializer &rez, AddressSpaceID target, - bool need_reference); + virtual void pack_expression(Serializer &rez, AddressSpaceID target); virtual void pack_expression_value(Serializer &rez,AddressSpaceID target); public: #ifdef DEBUG_LEGION diff --git a/runtime/legion/region_tree.inl b/runtime/legion/region_tree.inl index d0c57c4c7f..6325a6e2a7 100644 --- a/runtime/legion/region_tree.inl +++ b/runtime/legion/region_tree.inl @@ -1534,7 +1534,7 @@ namespace Legion { //-------------------------------------------------------------------------- template void IndexSpaceOperationT::pack_expression(Serializer &rez, - AddressSpaceID target, bool need_reference) + AddressSpaceID target) //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION @@ -1543,33 +1543,26 @@ namespace Legion { if (target == this->local_space) { rez.serialize(true/*local*/); - rez.serialize(need_reference); rez.serialize(this); - // Add a live expression reference to keep this live through the message - if (need_reference) - this->add_base_expression_reference(LIVE_EXPR_REF); + this->add_base_expression_reference(LIVE_EXPR_REF); } else if (target == this->owner_space) { rez.serialize(true/*local*/); - rez.serialize(need_reference); rez.serialize(origin_expr); // Add a reference here that we'll remove after we've added a reference // on the target space expression - if (need_reference) - this->add_base_expression_reference(REMOTE_DID_REF); + this->add_base_expression_reference(REMOTE_DID_REF); } else { rez.serialize(false/*local*/); rez.serialize(false/*index space*/); - rez.serialize(need_reference); rez.serialize(expr_id); rez.serialize(origin_expr); // Add a reference here that we'll remove after we've added a reference // on the target space expression - if (need_reference) - this->add_base_expression_reference(REMOTE_DID_REF); + this->add_base_expression_reference(REMOTE_DID_REF); } } From e5146cea1a63799a6b846e8f1c0b8bfa2d92bae8 Mon Sep 17 00:00:00 2001 From: Mike Bauer Date: Thu, 16 Dec 2021 00:12:29 -0800 Subject: [PATCH 8/9] legion: more refactoring of distributed index space reference counting to keep index space nodes live until the application removes its references --- runtime/legion/legion_context.cc | 6 +- runtime/legion/legion_ops.cc | 3 +- runtime/legion/region_tree.cc | 205 ++++++++++++++++++++++--------- runtime/legion/region_tree.h | 32 ++++- runtime/legion/runtime.cc | 10 +- runtime/legion/runtime.h | 3 +- 6 files changed, 185 insertions(+), 74 deletions(-) diff --git a/runtime/legion/legion_context.cc b/runtime/legion/legion_context.cc index 7e4fc5daf2..b1f45f135d 100644 --- a/runtime/legion/legion_context.cc +++ b/runtime/legion/legion_context.cc @@ -993,7 +993,8 @@ namespace Legion { REPORT_LEGION_WARNING(LEGION_WARNING_LEAKED_RESOURCE, "Index space %x was leaked out of task tree rooted by task %s", it->first.id, get_task_name()) - runtime->forest->destroy_index_space(it->first, preconditions); + runtime->forest->destroy_index_space(it->first, + runtime->address_space, preconditions); } created_index_spaces.clear(); } @@ -11115,7 +11116,8 @@ namespace Legion { handle.id, get_task_name(), get_unique_id()); #endif std::set preconditions; - runtime->forest->destroy_index_space(handle, preconditions); + runtime->forest->destroy_index_space(handle, + runtime->address_space, preconditions); if (!preconditions.empty()) { AutoLock l_lock(leaf_lock); diff --git a/runtime/legion/legion_ops.cc b/runtime/legion/legion_ops.cc index fb49efc6dd..5e33ce7e98 100644 --- a/runtime/legion/legion_ops.cc +++ b/runtime/legion/legion_ops.cc @@ -8542,7 +8542,8 @@ namespace Legion { #ifdef DEBUG_LEGION assert(deletion_req_indexes.empty()); #endif - runtime->forest->destroy_index_space(index_space, preconditions); + runtime->forest->destroy_index_space(index_space, + runtime->address_space, preconditions); if (!sub_partitions.empty()) { for (std::vector::const_iterator it = diff --git a/runtime/legion/region_tree.cc b/runtime/legion/region_tree.cc index 493fcda22d..d6748f57d4 100644 --- a/runtime/legion/region_tree.cc +++ b/runtime/legion/region_tree.cc @@ -484,20 +484,23 @@ namespace Legion { //-------------------------------------------------------------------------- void RegionTreeForest::destroy_index_space(IndexSpace handle, - std::set &applied) + AddressSpaceID source, std::set &applied) //-------------------------------------------------------------------------- { - const AddressSpaceID owner_space = - IndexSpaceNode::get_owner_space(handle, runtime); - if (owner_space == runtime->address_space) + IndexSpaceNode *node = get_node(handle); + WrapperReferenceMutator mutator(applied); + if (node->is_owner()) { - IndexSpaceNode *node = get_node(handle); - WrapperReferenceMutator mutator(applied); + node->invalidate_root(source, applied); if (node->remove_base_valid_ref(APPLICATION_REF, &mutator)) delete node; } else - runtime->send_index_space_destruction(handle, owner_space, applied); + { + runtime->send_index_space_destruction(handle,node->owner_space,applied); + if (node->remove_base_valid_ref(REMOTE_DID_REF, &mutator)) + delete node; + } } //-------------------------------------------------------------------------- @@ -3511,7 +3514,7 @@ namespace Legion { ApEvent is_ready, IndexSpaceExprID expr_id, std::set *applied, - bool add_remote_reference, + bool add_root_reference, unsigned depth) //-------------------------------------------------------------------------- { @@ -3557,19 +3560,37 @@ namespace Legion { // If we are the root then the valid ref comes from the application // Otherwise the valid ref comes from parent partition if (!result->is_owner()) - { - // We only add this if requested - if (add_remote_reference) - result->add_base_gc_ref(REMOTE_DID_REF, &mutator); - } - else if (parent == NULL) - result->add_base_valid_ref(APPLICATION_REF, &mutator); - else - result->add_nested_valid_ref(parent->did, &mutator); + // Always add a base gc ref for all index spaces + result->add_base_gc_ref(REMOTE_DID_REF, &mutator); result->register_with_runtime(&mutator); if (parent != NULL) - parent->add_child(result); - } + { +#ifdef DEBUG_LEGION + assert(!add_root_reference); +#endif + // Always add a valid reference from the parent + result->add_nested_valid_ref(parent->did, &mutator); + // Check to see if the parent is still tree valid + if (!parent->add_child(result)) + { + result->invalidate_tree(); + // If the parent is tree invalid then remove the reference + result->remove_nested_valid_ref(parent->did, &mutator); + } + } + else + { + if (result->is_owner()) + { +#ifdef DEBUG_LEGION + assert(!add_root_reference); +#endif + result->add_base_valid_ref(APPLICATION_REF, &mutator); + } + else if (add_root_reference) + result->add_base_valid_ref(REMOTE_DID_REF, &mutator); + } + } if (local_initialized.exists()) { if (!local_applied.empty()) @@ -3644,13 +3665,26 @@ namespace Legion { // Otherwise the valid ref comes from parent partition if (!result->is_owner()) result->add_base_gc_ref(REMOTE_DID_REF, &mutator); - else if (parent == NULL) - result->add_base_valid_ref(APPLICATION_REF, &mutator); - else - result->add_nested_valid_ref(parent->did, &mutator); result->register_with_runtime(&mutator); if (parent != NULL) - parent->add_child(result); + { + // Always add a valid reference from the parent + result->add_nested_valid_ref(parent->did, &mutator); + // Check to see if the parent is still tree valid + if (!parent->add_child(result)) + { + result->invalidate_tree(); + // If the parent is tree invalid then remove the reference + result->remove_nested_valid_ref(parent->did, &mutator); + } + } + else + { +#ifdef DEBUG_LEGION + assert(result->is_owner()); +#endif + result->add_base_valid_ref(APPLICATION_REF, &mutator); + } } if (local_initialized.exists()) { @@ -7918,10 +7952,11 @@ namespace Legion { send_references((parent != NULL) ? 1 : 0), realm_index_space_set(Runtime::create_rt_user_event()), tight_index_space_set(Runtime::create_rt_user_event()), - tight_index_space(false), tree_valid(is_owner()) + tight_index_space(false), tree_valid(true), #ifdef DEBUG_LEGION - , tree_active(true) + tree_active(true), #endif + root_valid(parent == NULL) //-------------------------------------------------------------------------- { #ifdef DEBUG_LEGION @@ -8009,10 +8044,13 @@ namespace Legion { { if (is_owner()) { - AutoLock n_lock(node_lock); - // First time we become invalid then the tree is no longer valid - // Any later valid states are just for expression references - tree_valid = false; + if (parent == NULL) + { + // If we're a root index space node and this is the first time + // we have become invalid then the tree is no longer valid + AutoLock n_lock(node_lock); + tree_valid = false; + } } else send_remote_valid_decrement(owner_space, mutator, @@ -8673,11 +8711,12 @@ namespace Legion { // At this point we are the owner #ifdef DEBUG_LEGION assert(is_owner()); + assert(tree_active); #endif bool pack_space = false; bool still_valid = false; bool has_reference = false; - bool add_remote_reference = false; + bool add_root_reference = false; // Do our check to see if we're still valid { AutoLock n_lock(node_lock); @@ -8711,13 +8750,14 @@ namespace Legion { if (tree_valid && ((parent == NULL) || (send_references > 0))) { still_valid = true; - add_remote_reference = true; // Grab a reference on the parent to keep it from being deleted if (parent != NULL) { send_references++; has_reference = true; } + else if (root_valid) + add_root_reference = true; } else if (above) { @@ -8729,13 +8769,6 @@ namespace Legion { send_precondition = finder->second; return false; } - else if (tree_valid || (send_references > 0)) - { - // Technically this invalid, but we still need to send a - // remote reference because we haven't issued the invalidates - // yet so we need one to be there when it arrives - add_remote_reference = true; - } // Record this as an effect for when the node is no longer valid #ifdef DEBUG_LEGION assert(send_effects.find(target) == send_effects.end()); @@ -8777,7 +8810,7 @@ namespace Legion { } // Record that we're going to send this node nodes_to_send.emplace_back(SendNodeRecord(this, still_valid, - add_remote_reference, pack_space, has_reference)); + add_root_reference, pack_space, has_reference)); return still_valid; } @@ -8808,7 +8841,7 @@ namespace Legion { rez.serialize(expr_id); rez.serialize(initialized); rez.serialize(depth); - rez.serialize(record.add_remote_reference); + rez.serialize(record.add_root_reference); if (record.pack_space) pack_index_space(rez, true/*include size*/); else @@ -8836,18 +8869,61 @@ namespace Legion { } //-------------------------------------------------------------------------- - void IndexSpaceNode::remove_send_reference(void) + void IndexSpaceNode::invalidate_tree(void) //-------------------------------------------------------------------------- { +#ifdef DEBUG_LEGION + assert(parent != NULL); +#endif bool remove_reference; { AutoLock n_lock(node_lock); +#ifdef DEBUG_LEGION + assert(tree_valid); +#endif + tree_valid = false; remove_reference = (--send_references == 0); } if (remove_reference && parent->remove_nested_resource_ref(did)) delete parent; } + //-------------------------------------------------------------------------- + void IndexSpaceNode::InvalidateRootFunctor::apply(AddressSpaceID target) + //-------------------------------------------------------------------------- + { + if (target == source) + return; + std::map::const_iterator finder = + effects.find(target); +#ifdef DEBUG_LEGION + assert(finder != effects.end()); +#endif + node->send_remote_valid_decrement(target, &mutator, finder->second); + } + + //-------------------------------------------------------------------------- + void IndexSpaceNode::invalidate_root(AddressSpaceID source, + std::set &applied) + //-------------------------------------------------------------------------- + { +#ifdef DEBUG_LEGION + assert(is_owner()); +#endif + AutoLock n_lock(node_lock); +#ifdef DEBUG_LEGION + assert(root_valid); +#endif + root_valid = false; + if (has_remote_instances()) + { + WrapperReferenceMutator mutator(applied); + InvalidateRootFunctor functor(source, this, mutator, + runtime, send_effects); + map_over_remote_instances(functor); + } + } + //-------------------------------------------------------------------------- /*static*/ void IndexSpaceNode::handle_node_creation( RegionTreeForest *context, Deserializer &derez, AddressSpaceID source) @@ -8870,8 +8946,8 @@ namespace Legion { derez.deserialize(initialized); unsigned depth; derez.deserialize(depth); - bool is_remote_valid; - derez.deserialize(is_remote_valid); + bool add_root_reference; + derez.deserialize(add_root_reference); size_t index_space_size; derez.deserialize(index_space_size); const void *index_space_ptr = @@ -8883,7 +8959,7 @@ namespace Legion { true/*can fail*/, true/*local only*/); IndexSpaceNode *node = context->create_node(handle, index_space_ptr, false/*is domain*/, parent_node, color, did, initialized, - ready_event, expr_id, NULL/*applied*/, is_remote_valid, depth); + ready_event, expr_id, NULL/*applied*/, add_root_reference, depth); #ifdef DEBUG_LEGION assert(node != NULL); #endif @@ -9476,7 +9552,7 @@ namespace Legion { total_children(color_sp->get_volume()), max_linearized_color(color_sp->get_max_linearized_color()), partition_ready(part_ready), partial_pending(partial), disjoint(dis), - has_complete(comp >= 0), complete(comp != 0), tree_valid(is_owner()), + has_complete(comp >= 0), complete(comp != 0), tree_valid(true), send_count(0) //-------------------------------------------------------------------------- { @@ -9632,13 +9708,6 @@ namespace Legion { { if (is_owner()) { - { - AutoLock n_lock(node_lock); -#ifdef DEBUG_LEGION - assert(tree_valid); -#endif - tree_valid = false; - } // Remove gc references from our remote nodes if (has_remote_instances()) { @@ -9675,12 +9744,25 @@ namespace Legion { // We still hold resource references to the node so we don't need to // worry about the child nodes being deleted parent->remove_child(color); - for (std::map::const_iterator it = - color_map.begin(); it != color_map.end(); it++) + std::vector to_invalidate; { - it->second->remove_send_reference(); - if (it->second->is_owner()) - it->second->remove_nested_valid_ref(did, mutator); + AutoLock n_lock(node_lock); +#ifdef DEBUG_LEGION + assert(tree_valid); +#endif + tree_valid = false; + to_invalidate.reserve(color_map.size()); + for (std::map::const_iterator it = + color_map.begin(); it != color_map.end(); it++) + to_invalidate.push_back(it->second); + } + for (std::vector::const_iterator it = + to_invalidate.begin(); it != to_invalidate.end(); it++) + { + (*it)->invalidate_tree(); + // Remove the nested valid reference on this index space node + if ((*it)->remove_nested_valid_ref(did, mutator)) + assert(false); // still holding resource ref so should never be hit } if (!partition_trackers.empty()) { @@ -10023,14 +10105,16 @@ namespace Legion { } //-------------------------------------------------------------------------- - void IndexPartNode::add_child(IndexSpaceNode *child) + bool IndexPartNode::add_child(IndexSpaceNode *child) //-------------------------------------------------------------------------- { // This child should live as long as we are alive child->add_nested_resource_ref(did); RtUserEvent to_trigger; + bool result; { AutoLock n_lock(node_lock); + result = tree_valid; #ifdef DEBUG_LEGION assert(color_map.find(child->color) == color_map.end()); #endif @@ -10038,11 +10122,12 @@ namespace Legion { std::map::iterator finder = pending_child_map.find(child->color); if (finder == pending_child_map.end()) - return; + return result; to_trigger = finder->second; pending_child_map.erase(finder); } Runtime::trigger_event(to_trigger); + return result; } //-------------------------------------------------------------------------- diff --git a/runtime/legion/region_tree.h b/runtime/legion/region_tree.h index 261ae001e7..658ba6676d 100644 --- a/runtime/legion/region_tree.h +++ b/runtime/legion/region_tree.h @@ -207,7 +207,7 @@ namespace Legion { std::set &safe_events); void compute_partition_disjointness(IndexPartition handle, RtUserEvent ready_event); - void destroy_index_space(IndexSpace handle, + void destroy_index_space(IndexSpace handle, AddressSpaceID source, std::set &preconditions); void destroy_index_partition(IndexPartition handle, std::set &preconditions); @@ -668,7 +668,7 @@ namespace Legion { ApEvent is_ready = ApEvent::NO_AP_EVENT, IndexSpaceExprID expr_id = 0, std::set *applied = NULL, - bool add_remote_reference = true, + bool add_root_reference = false, unsigned depth = UINT_MAX); IndexSpaceNode* create_node(IndexSpace is, const void *realm_is, IndexPartNode *par, LegionColor color, @@ -1861,12 +1861,12 @@ namespace Legion { public: SendNodeRecord(IndexTreeNode *n, bool valid = false, bool add = false, bool pack = false, bool has_ref = false) - : node(n), still_valid(valid), add_remote_reference(add), + : node(n), still_valid(valid), add_root_reference(add), pack_space(pack), has_reference(has_ref) { } public: IndexTreeNode *node; bool still_valid; - bool add_remote_reference; + bool add_root_reference; bool pack_space; bool has_reference; }; @@ -2003,6 +2003,21 @@ namespace Legion { ReferenceMutator *const mutator; std::map &send_effects; }; + class InvalidateRootFunctor { + public: + InvalidateRootFunctor(AddressSpaceID src, IndexSpaceNode *n, + ReferenceMutator &m, Runtime *rt, + const std::map &e) + : source(src), node(n), runtime(rt), mutator(m), effects(e) { } + public: + void apply(AddressSpaceID target); + public: + const AddressSpaceID source; + IndexSpaceNode *const node; + Runtime *const runtime; + ReferenceMutator &mutator; + const std::map &effects; + }; public: IndexSpaceNode(RegionTreeForest *ctx, IndexSpace handle, IndexPartNode *parent, LegionColor color, @@ -2065,7 +2080,9 @@ namespace Legion { const bool above = false); virtual void pack_node(Serializer &rez, AddressSpaceID target, const SendNodeRecord &record); - void remove_send_reference(void); + void invalidate_tree(void); + void invalidate_root(AddressSpaceID source, + std::set &applied); static void handle_node_creation(RegionTreeForest *context, Deserializer &derez, AddressSpaceID source); @@ -2273,6 +2290,9 @@ namespace Legion { // Keep track of whether we are active, should only happen once bool tree_active; #endif + // Keep track of whether we've had our application + // reference removed if this is a root node + bool root_valid; }; /** @@ -2960,7 +2980,7 @@ namespace Legion { public: bool has_color(const LegionColor c); IndexSpaceNode* get_child(const LegionColor c, RtEvent *defer = NULL); - void add_child(IndexSpaceNode *child); + bool add_child(IndexSpaceNode *child); void add_tracker(PartitionTracker *tracker); size_t get_num_children(void) const; void get_subspace_preconditions(std::set &preconditions); diff --git a/runtime/legion/runtime.cc b/runtime/legion/runtime.cc index 18ba843816..b38d53bd92 100644 --- a/runtime/legion/runtime.cc +++ b/runtime/legion/runtime.cc @@ -8303,7 +8303,8 @@ namespace Legion { } case INDEX_SPACE_DESTRUCTION_MESSAGE: { - runtime->handle_index_space_destruction(derez); + runtime->handle_index_space_destruction(derez, + remote_address_space); break; } case INDEX_PARTITION_DESTRUCTION_MESSAGE: @@ -18061,7 +18062,8 @@ namespace Legion { } //-------------------------------------------------------------------------- - void Runtime::handle_index_space_destruction(Deserializer &derez) + void Runtime::handle_index_space_destruction(Deserializer &derez, + AddressSpaceID source) //-------------------------------------------------------------------------- { DerezCheck z(derez); @@ -18073,7 +18075,7 @@ namespace Legion { assert(done.exists()); #endif std::set applied; - forest->destroy_index_space(handle, applied); + forest->destroy_index_space(handle, source, applied); if (!applied.empty()) Runtime::trigger_event(done, Runtime::merge_events(applied)); else @@ -20369,7 +20371,7 @@ namespace Legion { std::set applied; for (std::map,IndexSpace>::const_iterator it = index_slice_spaces.begin(); it != index_slice_spaces.end(); it++) - forest->destroy_index_space(it->second, applied); + forest->destroy_index_space(it->second, address_space, applied); // If there are still any layout constraints that the application // failed to remove its references to then we can remove the reference // for them and make sure it's effects propagate diff --git a/runtime/legion/runtime.h b/runtime/legion/runtime.h index e9097c7f3f..f2ca2f1135 100644 --- a/runtime/legion/runtime.h +++ b/runtime/legion/runtime.h @@ -2520,7 +2520,8 @@ namespace Legion { AddressSpaceID source); void handle_top_level_region_return(Deserializer &derez, AddressSpaceID source); - void handle_index_space_destruction(Deserializer &derez); + void handle_index_space_destruction(Deserializer &derez, + AddressSpaceID source); void handle_index_partition_destruction(Deserializer &derez); void handle_field_space_destruction(Deserializer &derez); void handle_logical_region_destruction(Deserializer &derez); From cf4f5e7ddc071c78875f3b9225f533c6c83fe5d0 Mon Sep 17 00:00:00 2001 From: Mike Bauer Date: Fri, 17 Dec 2021 15:53:27 -0800 Subject: [PATCH 9/9] legion: cleanup of distributed expressions --- runtime/legion/garbage_collection.cc | 75 ---------------------------- runtime/legion/garbage_collection.h | 21 +------- runtime/legion/legion_analysis.cc | 6 +-- runtime/legion/legion_context.cc | 2 +- runtime/legion/legion_instances.cc | 8 +-- runtime/legion/legion_ops.h | 3 +- runtime/legion/legion_tasks.cc | 2 +- runtime/legion/region_tree.cc | 40 +++++++-------- runtime/legion/region_tree.h | 2 +- runtime/legion/runtime.cc | 8 +-- 10 files changed, 36 insertions(+), 131 deletions(-) diff --git a/runtime/legion/garbage_collection.cc b/runtime/legion/garbage_collection.cc index d3b185827b..9c060ef24b 100644 --- a/runtime/legion/garbage_collection.cc +++ b/runtime/legion/garbage_collection.cc @@ -31,7 +31,6 @@ namespace Legion { //-------------------------------------------------------------------------- LocalReferenceMutator::LocalReferenceMutator( const LocalReferenceMutator &rhs) - : waiter(rhs.waiter) //-------------------------------------------------------------------------- { // should never be called @@ -44,9 +43,6 @@ namespace Legion { { if (!mutation_effects.empty()) { -#ifdef DEBUG_LEGION - assert(waiter); -#endif RtEvent wait_on = Runtime::merge_events(mutation_effects); wait_on.wait(); } @@ -73,9 +69,6 @@ namespace Legion { RtEvent LocalReferenceMutator::get_done_event(void) //-------------------------------------------------------------------------- { -#ifdef DEBUG_LEGION - assert(!waiter); -#endif if (mutation_effects.empty()) return RtEvent::NO_RT_EVENT; RtEvent result = Runtime::merge_events(mutation_effects); @@ -1821,23 +1814,6 @@ namespace Legion { #ifdef DEBUG_LEGION assert(count != 0); assert(registered_with_runtime); -#endif -#if 0 - // If there is no mutator or it is a non-waiting mutator then we - // can buffer this up in the implicit reference tracker and send it - // at the end of the runtime call or meta-task - if ((mutator == NULL) || !mutator->is_waiting_mutator()) - { - if (implicit_reference_tracker == NULL) - implicit_reference_tracker = new ImplicitReferenceTracker; - const RtEvent send_event = - implicit_reference_tracker->record_valid_increment(did, target, - precondition, count); - if (mutator != NULL) - mutator->record_reference_mutation_effect( - implicit_reference_tracker->get_effects_event()); - return send_event; - } #endif RtUserEvent done_event; if (mutator != NULL) @@ -1875,23 +1851,6 @@ namespace Legion { assert(count != 0); assert(registered_with_runtime); #endif -#if 0 - // If there is no mutator or it is a non-waiting mutator then we - // can buffer this up in the implicit reference tracker and send it - // at the end of the runtime call or meta-task - if ((mutator == NULL) || !mutator->is_waiting_mutator()) - { - if (implicit_reference_tracker == NULL) - implicit_reference_tracker = new ImplicitReferenceTracker; - const RtEvent send_event = - implicit_reference_tracker->record_valid_decrement(did, target, - precondition, count); - if (mutator != NULL) - mutator->record_reference_mutation_effect( - implicit_reference_tracker->get_effects_event()); - return send_event; - } -#endif RtUserEvent done_event; if (mutator != NULL) { @@ -1927,23 +1886,6 @@ namespace Legion { #ifdef DEBUG_LEGION assert(count != 0); assert(registered_with_runtime); -#endif -#if 0 - // If there is no mutator or it is a non-waiting mutator then we - // can buffer this up in the implicit reference tracker and send it - // at the end of the runtime call or meta-task - if ((mutator == NULL) || !mutator->is_waiting_mutator()) - { - if (implicit_reference_tracker == NULL) - implicit_reference_tracker = new ImplicitReferenceTracker; - const RtEvent send_event = - implicit_reference_tracker->record_gc_increment(did, target, - precondition, count); - if (mutator != NULL) - mutator->record_reference_mutation_effect( - implicit_reference_tracker->get_effects_event()); - return send_event; - } #endif RtUserEvent done_event; if (mutator != NULL) @@ -1980,23 +1922,6 @@ namespace Legion { #ifdef DEBUG_LEGION assert(count != 0); assert(registered_with_runtime); -#endif -#if 0 - // If there is no mutator or it is a non-waiting mutator then we - // can buffer this up in the implicit reference tracker and send it - // at the end of the runtime call or meta-task - if ((mutator == NULL) || !mutator->is_waiting_mutator()) - { - if (implicit_reference_tracker == NULL) - implicit_reference_tracker = new ImplicitReferenceTracker; - const RtEvent send_event = - implicit_reference_tracker->record_gc_increment(did, target, - precondition, count); - if (mutator != NULL) - mutator->record_reference_mutation_effect( - implicit_reference_tracker->get_effects_event()); - return send_event; - } #endif RtUserEvent done_event; if (mutator != NULL) diff --git a/runtime/legion/garbage_collection.h b/runtime/legion/garbage_collection.h index ae2c09445d..7ac4fa1e79 100644 --- a/runtime/legion/garbage_collection.h +++ b/runtime/legion/garbage_collection.h @@ -182,7 +182,6 @@ namespace Legion { */ class ReferenceMutator { public: - virtual bool is_waiting_mutator(void) const = 0; virtual void record_reference_mutation_effect(RtEvent event) = 0; }; @@ -194,19 +193,17 @@ namespace Legion { */ class LocalReferenceMutator : public ReferenceMutator { public: - LocalReferenceMutator(bool wait) : waiter(wait) { } + LocalReferenceMutator(void) { } LocalReferenceMutator(const LocalReferenceMutator &rhs); ~LocalReferenceMutator(void); public: LocalReferenceMutator& operator=(const LocalReferenceMutator &rhs); public: - virtual bool is_waiting_mutator(void) const { return waiter; } virtual void record_reference_mutation_effect(RtEvent event); public: RtEvent get_done_event(void); private: std::set mutation_effects; - const bool waiter; }; /** @@ -222,7 +219,6 @@ namespace Legion { public: WrapperReferenceMutator& operator=(const WrapperReferenceMutator &rhs); public: - virtual bool is_waiting_mutator(void) const { return false; } virtual void record_reference_mutation_effect(RtEvent event); private: std::set &mutation_effects; @@ -245,21 +241,6 @@ namespace Legion { public: inline void record_live_expression(IndexSpaceExpression *expr) { live_expressions.emplace_back(expr); } -#if 0 - public: - RtEvent record_valid_increment(DistributedID did, - AddressSpaceID target, - unsigned count); - RtEvent record_valid_decrement(DistributedID did, - AddressSpaceID target, - unsigned count); - RtEvent record_gc_increment(DistributedID did, - AddressSpaceID target, - unsigned count); - RtEvent record_gc_decrement(DistributedID did, - AddressSpaceID target, - unsigned count); -#endif private: std::vector live_expressions; }; diff --git a/runtime/legion/legion_analysis.cc b/runtime/legion/legion_analysis.cc index ac0b07211b..b5c1a82c3a 100644 --- a/runtime/legion/legion_analysis.cc +++ b/runtime/legion/legion_analysis.cc @@ -10081,7 +10081,7 @@ namespace Legion { if (is_logical_owner() || initial_refinement) { // We're the owner so we can do the merge - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; for (FieldMaskSet::const_iterator it = new_views.begin(); it != new_views.end(); it++) if (valid_instances.insert(it->first, it->second)) @@ -10610,7 +10610,7 @@ namespace Legion { transition_event = RtUserEvent::NO_RT_USER_EVENT; } eq_state = MAPPING_STATE; - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; // Add references to all the views that we've loaded for (FieldMaskSet::const_iterator it = valid_instances.begin(); it != valid_instances.end(); it++) @@ -13885,7 +13885,7 @@ namespace Legion { const RemoteRefTaskArgs *rargs = (const RemoteRefTaskArgs*)args; if (rargs->done_event.exists()) { - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; if (rargs->add_references) { for (std::map::const_iterator it = diff --git a/runtime/legion/legion_context.cc b/runtime/legion/legion_context.cc index b1f45f135d..5f45e2b67c 100644 --- a/runtime/legion/legion_context.cc +++ b/runtime/legion/legion_context.cc @@ -5786,7 +5786,7 @@ namespace Legion { const DistributedID did = runtime->get_available_distributed_id(); FutureMapImpl *impl = new FutureMapImpl(this, runtime, did, runtime->address_space, RtEvent::NO_RT_EVENT); - LocalReferenceMutator mutator(true/*waiter*/); + LocalReferenceMutator mutator; for (std::map::const_iterator it = data.begin(); it != data.end(); it++) { diff --git a/runtime/legion/legion_instances.cc b/runtime/legion/legion_instances.cc index f2dffa957d..a6bf8ee726 100644 --- a/runtime/legion/legion_instances.cc +++ b/runtime/legion/legion_instances.cc @@ -3093,7 +3093,7 @@ namespace Legion { { RtUserEvent to_trigger; derez.deserialize(to_trigger); - LocalReferenceMutator mutator(false/*waiter*/);; + LocalReferenceMutator mutator; manager->activate_collective(&mutator); Runtime::trigger_event(to_trigger, mutator.get_done_event()); break; @@ -3102,7 +3102,7 @@ namespace Legion { { RtUserEvent to_trigger; derez.deserialize(to_trigger); - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; manager->deactivate_collective(&mutator); Runtime::trigger_event(to_trigger, mutator.get_done_event()); break; @@ -3111,7 +3111,7 @@ namespace Legion { { RtUserEvent to_trigger; derez.deserialize(to_trigger); - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; manager->validate_collective(&mutator); Runtime::trigger_event(to_trigger, mutator.get_done_event()); break; @@ -3120,7 +3120,7 @@ namespace Legion { { RtUserEvent to_trigger; derez.deserialize(to_trigger); - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; manager->invalidate_collective(&mutator); Runtime::trigger_event(to_trigger, mutator.get_done_event()); break; diff --git a/runtime/legion/legion_ops.h b/runtime/legion/legion_ops.h index 23fc1e8ea2..4f61311fc0 100644 --- a/runtime/legion/legion_ops.h +++ b/runtime/legion/legion_ops.h @@ -348,7 +348,6 @@ namespace Legion { const std::vector *dependences = NULL); public: // Inherited from ReferenceMutator - virtual bool is_waiting_mutator(void) const { return false; } virtual void record_reference_mutation_effect(RtEvent event); public: RtEvent execute_prepipeline_stage(GenerationID gen, @@ -618,7 +617,7 @@ namespace Legion { protected: static inline void add_launch_space_reference(IndexSpaceNode *node) { - LocalReferenceMutator mutator(true/*waiter*/); + LocalReferenceMutator mutator; node->add_base_valid_ref(CONTEXT_REF, &mutator); } static inline bool remove_launch_space_reference(IndexSpaceNode *node) diff --git a/runtime/legion/legion_tasks.cc b/runtime/legion/legion_tasks.cc index 7d7b4f3345..e7b3d69192 100644 --- a/runtime/legion/legion_tasks.cc +++ b/runtime/legion/legion_tasks.cc @@ -9473,7 +9473,7 @@ namespace Legion { #ifdef DEBUG_LEGION assert(finder != future_handles->handles.end()); #endif - LocalReferenceMutator mutator(false/*not waiting*/); + LocalReferenceMutator mutator; FutureImpl *impl = runtime->find_or_create_future(finder->second, parent_ctx->get_context_uid(), &mutator); if (functor != NULL) diff --git a/runtime/legion/region_tree.cc b/runtime/legion/region_tree.cc index d6748f57d4..2fbc07543d 100644 --- a/runtime/legion/region_tree.cc +++ b/runtime/legion/region_tree.cc @@ -5665,7 +5665,7 @@ namespace Legion { // Add the live reference if (mutator == NULL) { - LocalReferenceMutator local_mutator(true/*waiter*/); + LocalReferenceMutator local_mutator; result->add_base_expression_reference(LIVE_EXPR_REF, &local_mutator); } else @@ -5705,7 +5705,7 @@ namespace Legion { } if (expressions.empty()) return *(exprs.begin()); - LocalReferenceMutator local_mutator(true/*waiter*/); + LocalReferenceMutator local_mutator; if (expressions.size() == 1) { IndexSpaceExpression *result = expressions.back(); @@ -5984,7 +5984,7 @@ namespace Legion { // Add the live reference if (mutator == NULL) { - LocalReferenceMutator local_mutator(true/*waiter*/); + LocalReferenceMutator local_mutator; result->add_base_expression_reference(LIVE_EXPR_REF, &local_mutator); } else @@ -6028,7 +6028,7 @@ namespace Legion { // remove duplicates std::vector::iterator last = std::unique(expressions.begin(), expressions.end()); - LocalReferenceMutator local_mutator(true/*waiter*/); + LocalReferenceMutator local_mutator; if (last != expressions.end()) { expressions.erase(last, expressions.end()); @@ -6383,7 +6383,7 @@ namespace Legion { } if (mutator == NULL) { - LocalReferenceMutator local_mutator(true/*waiter*/); + LocalReferenceMutator local_mutator; result->add_base_expression_reference(LIVE_EXPR_REF, &local_mutator); } else @@ -6607,7 +6607,7 @@ namespace Legion { if (pending.is_index_space) { IndexSpaceNode *node = get_node(pending.handle); - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; node->add_base_expression_reference(LIVE_EXPR_REF, &mutator); const RtEvent added = mutator.get_done_event(); // Special case here: if the source was the owner and we didn't @@ -6643,7 +6643,7 @@ namespace Legion { #else IndexSpaceOperation *op = static_cast(result); #endif - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; result->add_base_expression_reference(LIVE_EXPR_REF, &mutator); const RtEvent added = mutator.get_done_event(); // Special case here: if the source was the owner and we didn't @@ -6914,7 +6914,7 @@ namespace Legion { #endif // Make this valid and then send the removal of the // remote did expression - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; op->add_base_expression_reference(LIVE_EXPR_REF, &mutator); // Always need to send this reference removal back immediately // in order to avoid reference counting deadlock @@ -6934,7 +6934,7 @@ namespace Legion { IndexSpace handle; derez.deserialize(handle); IndexSpaceNode *node = forest->get_node(handle); - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; node->add_base_expression_reference(LIVE_EXPR_REF, &mutator); const RtEvent added = mutator.get_done_event(); // Special case here: if the source was the owner and we didn't @@ -6966,7 +6966,7 @@ namespace Legion { #else IndexSpaceOperation *op = static_cast(result); #endif - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; result->add_base_expression_reference(LIVE_EXPR_REF, &mutator); const RtEvent added = mutator.get_done_event(); // Special case here: if the source was the owner and we didn't @@ -7010,7 +7010,7 @@ namespace Legion { #endif // Make this valid and then send the removal of the // remote did expression - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; op->add_base_expression_reference(LIVE_EXPR_REF, &mutator); // Always need to send this reference removal back immediately // in order to avoid reference counting deadlock @@ -7033,7 +7033,7 @@ namespace Legion { pending.source = source; return node; } - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; node->add_base_expression_reference(LIVE_EXPR_REF, &mutator); const RtEvent added = mutator.get_done_event(); // Special case here: if the source was the owner and we didn't @@ -7068,7 +7068,7 @@ namespace Legion { #else IndexSpaceOperation *op = static_cast(result); #endif - LocalReferenceMutator mutator(false/*waiter*/); + LocalReferenceMutator mutator; result->add_base_expression_reference(LIVE_EXPR_REF, &mutator); const RtEvent added = mutator.get_done_event(); // Special case here: if the source was the owner and we didn't @@ -7264,7 +7264,7 @@ namespace Legion { { if (mutator == NULL) { - LocalReferenceMutator local_mutator(true/*waiter*/); + LocalReferenceMutator local_mutator; add_base_valid_ref(source, &local_mutator, count); } else @@ -7287,7 +7287,7 @@ namespace Legion { { if (mutator == NULL) { - LocalReferenceMutator local_mutator(true/*waiter*/); + LocalReferenceMutator local_mutator; add_nested_valid_ref(source, &local_mutator, count); } else @@ -8789,7 +8789,7 @@ namespace Legion { // If this is above then we don't care about it if it // is not still valid bool remove_reference = false; - LocalReferenceMutator mutator(true/*waiter*/); + LocalReferenceMutator mutator; if (has_reference) { AutoLock n_lock(node_lock); @@ -8823,7 +8823,7 @@ namespace Legion { assert(record.node == this); #endif bool remove_reference = false; - LocalReferenceMutator mutator(true/*waiter*/); + LocalReferenceMutator mutator; { AutoLock n_lock(node_lock); { @@ -9281,7 +9281,7 @@ namespace Legion { // This could be a performance bug since it will block if we // have to send a reference to a remote node, but that should // never actually happen - LocalReferenceMutator mutator(true/*waiter*/); + LocalReferenceMutator mutator; add_base_gc_ref(REMOTE_DID_REF, &mutator); } @@ -9320,7 +9320,7 @@ namespace Legion { { if (mutator == NULL) { - LocalReferenceMutator local_mutator(true/*waiter*/); + LocalReferenceMutator local_mutator; add_base_valid_ref(source, &local_mutator, count); } else @@ -9343,7 +9343,7 @@ namespace Legion { { if (mutator == NULL) { - LocalReferenceMutator local_mutator(true/*waiter*/); + LocalReferenceMutator local_mutator; add_nested_valid_ref(source, &local_mutator, count); } else diff --git a/runtime/legion/region_tree.h b/runtime/legion/region_tree.h index 658ba6676d..4739bb69a2 100644 --- a/runtime/legion/region_tree.h +++ b/runtime/legion/region_tree.h @@ -1379,7 +1379,7 @@ namespace Legion { { if (m == NULL) { - LocalReferenceMutator local_mutator(true/*waiting*/); + LocalReferenceMutator local_mutator; expr->add_base_expression_reference(LIVE_EXPR_REF, &local_mutator); } else diff --git a/runtime/legion/runtime.cc b/runtime/legion/runtime.cc index b38d53bd92..f7ad941ae1 100644 --- a/runtime/legion/runtime.cc +++ b/runtime/legion/runtime.cc @@ -5254,7 +5254,7 @@ namespace Legion { bool remove_duplicate = false; if (success.load()) { - LocalReferenceMutator local_mutator(true/*waiter*/); + LocalReferenceMutator local_mutator; // Add our local reference manager->add_base_valid_ref(NEVER_GC_REF, &local_mutator); const RtEvent reference_effects = local_mutator.get_done_event(); @@ -5969,7 +5969,7 @@ namespace Legion { // and then remove the remote DID if (acquire) { - LocalReferenceMutator local_mutator(false/*waiter*/); + LocalReferenceMutator local_mutator; manager->add_base_valid_ref(MAPPING_ACQUIRE_REF, &local_mutator); const RtEvent reference_effects = local_mutator.get_done_event(); manager->send_remote_valid_decrement(source, NULL, @@ -6103,7 +6103,7 @@ namespace Legion { // and then remove the remote DID if (acquire) { - LocalReferenceMutator local_mutator(true/*waiter*/); + LocalReferenceMutator local_mutator; manager->add_base_valid_ref(MAPPING_ACQUIRE_REF, &local_mutator); const RtEvent reference_effects = local_mutator.get_done_event(); manager->send_remote_valid_decrement(source, NULL, @@ -6323,7 +6323,7 @@ namespace Legion { (*target)[index] = true; PhysicalManager *manager; derez.deserialize(manager); - LocalReferenceMutator local_mutator(false/*waiter*/); + LocalReferenceMutator local_mutator; manager->add_base_valid_ref(MAPPING_ACQUIRE_REF, &local_mutator); const RtEvent reference_effects = local_mutator.get_done_event(); manager->send_remote_valid_decrement(source, NULL, reference_effects);