From f74f2c5ccd6b9ec7eac06831078bbdf9c99e5bc4 Mon Sep 17 00:00:00 2001 From: Sylvan Clebsch Date: Thu, 2 Mar 2017 17:57:22 +0000 Subject: [PATCH 1/4] Assume nominal subtyping (#1620) * Assume nominal subtyping Previously, we assumed nominal <: structural to avoid infinite recursion when checking structural types. This change assumes nominal <: nominal (i.e. assumes more often) in order to avoid the same problem with other edge cases, such as a constraint bound to itself. * remove old assumes --- src/libponyc/type/subtype.c | 38 ++++++++++++++--------------- test/libponyc/type_check_subtype.cc | 19 +++++++++++++++ 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/libponyc/type/subtype.c b/src/libponyc/type/subtype.c index 6dc89094bb0..a7e56ed5ef3 100644 --- a/src/libponyc/type/subtype.c +++ b/src/libponyc/type/subtype.c @@ -769,10 +769,6 @@ static bool is_nominal_sub_structural(ast_t* sub, ast_t* super, ast_t* sub_def = (ast_t*)ast_data(sub); ast_t* super_def = (ast_t*)ast_data(super); - // Add an assumption: sub <: super - if(push_assume(sub, super, opt)) - return true; - bool ret = true; ast_t* sub_typeargs = ast_childidx(sub, 2); @@ -838,7 +834,6 @@ static bool is_nominal_sub_structural(ast_t* sub, ast_t* super, super_member = ast_sibling(super_member); } - pop_assume(); return ret; } @@ -926,14 +921,8 @@ static bool is_entity_sub_trait(ast_t* sub, ast_t* super, return true; } -static bool is_struct_sub_trait(ast_t* sub, ast_t* super, check_cap_t check_cap, - errorframe_t* errorf, pass_opt_t* opt) +static bool is_struct_sub_trait(ast_t* sub, ast_t* super, errorframe_t* errorf) { - (void)check_cap; - - if(check_assume(sub, super, opt)) - return true; - struct_cant_be_x(sub, super, errorf, "a trait"); return false; } @@ -995,7 +984,7 @@ static bool is_nominal_sub_trait(ast_t* sub, ast_t* super, return is_entity_sub_trait(sub, super, check_cap, errorf, opt); case TK_STRUCT: - return is_struct_sub_trait(sub, super, check_cap, errorf, opt); + return is_struct_sub_trait(sub, super, errorf); case TK_TRAIT: return is_trait_sub_trait(sub, super, check_cap, errorf, opt); @@ -1013,8 +1002,13 @@ static bool is_nominal_sub_trait(ast_t* sub, ast_t* super, static bool is_nominal_sub_nominal(ast_t* sub, ast_t* super, check_cap_t check_cap, errorframe_t* errorf, pass_opt_t* opt) { + // Add an assumption: sub <: super + if(push_assume(sub, super, opt)) + return true; + // N k <: N' k' ast_t* super_def = (ast_t*)ast_data(super); + bool ret = false; switch(ast_id(super_def)) { @@ -1022,19 +1016,23 @@ static bool is_nominal_sub_nominal(ast_t* sub, ast_t* super, case TK_STRUCT: case TK_CLASS: case TK_ACTOR: - return is_nominal_sub_entity(sub, super, check_cap, errorf, opt); + ret = is_nominal_sub_entity(sub, super, check_cap, errorf, opt); + break; case TK_INTERFACE: - return is_nominal_sub_interface(sub, super, check_cap, errorf, opt); + ret = is_nominal_sub_interface(sub, super, check_cap, errorf, opt); + break; case TK_TRAIT: - return is_nominal_sub_trait(sub, super, check_cap, errorf, opt); + ret = is_nominal_sub_trait(sub, super, check_cap, errorf, opt); + break; - default: {} + default: + pony_assert(0); } - pony_assert(0); - return false; + pop_assume(); + return ret; } static bool is_nominal_sub_typeparam(ast_t* sub, ast_t* super, @@ -1515,7 +1513,7 @@ static bool is_arrow_sub_x(ast_t* sub, ast_t* super, check_cap_t check_cap, return false; } -bool is_x_sub_x(ast_t* sub, ast_t* super, check_cap_t check_cap, +static bool is_x_sub_x(ast_t* sub, ast_t* super, check_cap_t check_cap, errorframe_t* errorf, pass_opt_t* opt) { pony_assert(sub != NULL); diff --git a/test/libponyc/type_check_subtype.cc b/test/libponyc/type_check_subtype.cc index b269df9da76..6c604559031 100644 --- a/test/libponyc/type_check_subtype.cc +++ b/test/libponyc/type_check_subtype.cc @@ -1033,3 +1033,22 @@ TEST_F(SubTypeTest, IsBeSubFunTag) pass_opt_init(&opt); } + + +TEST_F(SubTypeTest, IsTypeparamSubIntersect) +{ + const char* src = + "class B\n" + " fun f[A: (I32 & Real[A])](a: A, b: (I32 & Real[A])) =>\n" + " None"; + + TEST_COMPILE(src); + + pass_opt_t opt; + pass_opt_init(&opt); + + ASSERT_TRUE(is_subtype(type_of("a"), type_of("b"), NULL, &opt)); + ASSERT_TRUE(is_subtype(type_of("b"), type_of("a"), NULL, &opt)); + + pass_opt_init(&opt); +} From 3f850edaf9e6a00f7c6b991fd8060521bb1af764 Mon Sep 17 00:00:00 2001 From: Benoit Vey Date: Thu, 2 Mar 2017 18:57:36 +0100 Subject: [PATCH 2/4] Free the cycle detector data structures (#1623) This change fixes a memory leak caused by the cycle detector not being entirely freed when stopping the runtime. --- src/libponyrt/gc/cycle.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/libponyrt/gc/cycle.c b/src/libponyrt/gc/cycle.c index 41a94287042..00d3a9a217f 100644 --- a/src/libponyrt/gc/cycle.c +++ b/src/libponyrt/gc/cycle.c @@ -675,22 +675,25 @@ static void final(pony_ctx_t* ctx, pony_actor_t* self) // Find block messages and invoke finalisers for those actors pony_msg_t* msg; - while((msg = ponyint_messageq_pop(&self->q)) != NULL) + do { - if(msg->id == ACTORMSG_BLOCK) + while((msg = ponyint_messageq_pop(&self->q)) != NULL) { - block_msg_t* m = (block_msg_t*)msg; + if(msg->id == ACTORMSG_BLOCK) + { + block_msg_t* m = (block_msg_t*)msg; - if(m->delta != NULL) - ponyint_deltamap_free(m->delta); + if(m->delta != NULL) + ponyint_deltamap_free(m->delta); - if(!ponyint_actor_pendingdestroy(m->actor)) - { - ponyint_actor_setpendingdestroy(m->actor); - ponyint_actor_final(ctx, m->actor); + if(!ponyint_actor_pendingdestroy(m->actor)) + { + ponyint_actor_setpendingdestroy(m->actor); + ponyint_actor_final(ctx, m->actor); + } } } - } + } while(!ponyint_messageq_markempty(&self->q)); detector_t* d = (detector_t*)self; size_t i = HASHMAP_BEGIN; @@ -707,6 +710,14 @@ static void final(pony_ctx_t* ctx, pony_actor_t* self) ponyint_actor_final(ctx, view->actor); } } + + i = HASHMAP_BEGIN; + while((view = ponyint_viewmap_next(&d->deferred, &i)) != NULL) + ponyint_viewmap_remove(&d->deferred, view); + + ponyint_viewmap_destroy(&d->deferred); + ponyint_viewmap_destroy(&d->views); + ponyint_perceivedmap_destroy(&d->perceived); } #ifndef NDEBUG @@ -908,6 +919,8 @@ void ponyint_cycle_terminate(pony_ctx_t* ctx) { pony_become(ctx, cycle_detector); final(ctx, cycle_detector); + ponyint_destroy(cycle_detector); + cycle_detector = NULL; } bool ponyint_is_cycle(pony_actor_t* actor) From 567400f1feb6f6448d4b1f961636c60cb7753980 Mon Sep 17 00:00:00 2001 From: Benoit Vey Date: Thu, 2 Mar 2017 18:58:05 +0100 Subject: [PATCH 3/4] Synchronise on success only in ponyint_messageq_pop (#1622) There is no need to synchronise on failure. Using less synchronisation can result in better performance, particularly on architectures with weak memory ordering such as ARM. --- src/libponyrt/actor/messageq.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libponyrt/actor/messageq.c b/src/libponyrt/actor/messageq.c index 04689f3405b..a91f090b324 100644 --- a/src/libponyrt/actor/messageq.c +++ b/src/libponyrt/actor/messageq.c @@ -80,15 +80,14 @@ bool ponyint_messageq_push(messageq_t* q, pony_msg_t* m) pony_msg_t* ponyint_messageq_pop(messageq_t* q) { pony_msg_t* tail = q->tail; - pony_msg_t* next = atomic_load_explicit(&tail->next, memory_order_acquire); -#ifdef USE_VALGRIND - ANNOTATE_HAPPENS_AFTER(&tail->next); -#endif + pony_msg_t* next = atomic_load_explicit(&tail->next, memory_order_relaxed); if(next != NULL) { q->tail = next; + atomic_thread_fence(memory_order_acquire); #ifdef USE_VALGRIND + ANNOTATE_HAPPENS_AFTER(&tail->next); ANNOTATE_HAPPENS_BEFORE_FORGET_ALL(tail); #endif ponyint_pool_free(tail->index, tail); From b7299f68ede502b0b950fc5b94ee9ff3cba65ed3 Mon Sep 17 00:00:00 2001 From: Benoit Vey Date: Fri, 3 Mar 2017 14:09:55 +0100 Subject: [PATCH 4/4] Load the initial tail non-atomically in ponyint_mpmcq_pop (#1624) This is a change originally suggested by @sylvanc in #1621. On x86, this results in less occurrences of the expensive cmpxchg16b instruction. --- src/libponyrt/sched/mpmcq.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libponyrt/sched/mpmcq.c b/src/libponyrt/sched/mpmcq.c index 018f7f66424..66fef64f2a5 100644 --- a/src/libponyrt/sched/mpmcq.c +++ b/src/libponyrt/sched/mpmcq.c @@ -93,10 +93,14 @@ void ponyint_mpmcq_push_single(mpmcq_t* q, void* data) void* ponyint_mpmcq_pop(mpmcq_t* q) { #ifdef PLATFORM_IS_X86 - PONY_ABA_PROTECTED(mpmcq_node_t*) cmp = bigatomic_load_explicit(&q->tail, - memory_order_relaxed); + PONY_ABA_PROTECTED(mpmcq_node_t*) cmp; PONY_ABA_PROTECTED(mpmcq_node_t*) xchg; mpmcq_node_t* tail; + // Load the tail non-atomically. If object and counter are out of sync, we'll + // do an additional CAS iteration which isn't less efficient than doing an + // atomic initial load. + cmp.object = q->tail.object; + cmp.counter = q->tail.counter; #else mpmcq_node_t* tail = atomic_load_explicit(&q->tail, memory_order_relaxed); #endif