From e15319fcdb5c99514cd63d7a02d04c97587e8853 Mon Sep 17 00:00:00 2001 From: Guilherme Andrade Date: Mon, 27 Jun 2016 22:09:37 +0100 Subject: Disable ets:select_replace/2 for bags The existing implementation presented both semantic inconsistencies and performance issues. --- erts/emulator/beam/erl_db.c | 8 +++++ erts/emulator/beam/erl_db_hash.c | 74 +++------------------------------------- 2 files changed, 12 insertions(+), 70 deletions(-) (limited to 'erts/emulator') diff --git a/erts/emulator/beam/erl_db.c b/erts/emulator/beam/erl_db.c index 81fe79a92e..d77c585628 100644 --- a/erts/emulator/beam/erl_db.c +++ b/erts/emulator/beam/erl_db.c @@ -2982,6 +2982,14 @@ BIF_RETTYPE ets_select_replace_2(BIF_ALIST_2) if ((tb = db_get_table(BIF_P, BIF_ARG_1, DB_WRITE, LCK_WRITE_REC)) == NULL) { BIF_ERROR(BIF_P, BADARG); } + + if (tb->common.status & DB_BAG) { + /* Bag implementation presented both semantic consistency + and performance issues */ + db_unlock(tb, LCK_WRITE_REC); + BIF_ERROR(BIF_P, BADARG); + } + safety = ITERATION_SAFETY(BIF_P,tb); if (safety == ITER_UNSAFE) { local_fix_table(tb); diff --git a/erts/emulator/beam/erl_db_hash.c b/erts/emulator/beam/erl_db_hash.c index a4ca6dce77..96d74bb050 100644 --- a/erts/emulator/beam/erl_db_hash.c +++ b/erts/emulator/beam/erl_db_hash.c @@ -2009,7 +2009,7 @@ static int db_select_replace_hash(Process *p, DbTableHash *tb = &tbl->hash; struct mp_info mpi; Uint slot_ix = 0; - HashDbTerm **current = NULL, **initial = NULL, **iter = NULL; + HashDbTerm **current = NULL; HashDbTerm *new = NULL, *next = NULL; HashValue hval = INVALID_HASH; unsigned current_list_pos = 0; @@ -2036,6 +2036,8 @@ static int db_select_replace_hash(Process *p, return RetVal; \ } while(0) + /* Bag implementation presented both semantic consistency and performance issues */ + ASSERT(!(tb->common.status & DB_BAG)); if ((errcode = analyze_pattern(tb, pattern, &mpi)) != DB_ERROR_NONE) { RET_TO_BIF(NIL,errcode); @@ -2060,7 +2062,6 @@ static int db_select_replace_hash(Process *p, } - initial = current; for(;;) { if ((*current) == NULL) { if (mpi.key_given) { /* Key is bound */ @@ -2084,43 +2085,10 @@ static int db_select_replace_hash(Process *p, } current = &BUCKET(tb,slot_ix); } - initial = current; } else if ((*current)->hvalue == INVALID_HASH) { current = &((*current)->next); } - else if (tb->common.status & DB_BAG) { - match_res = db_match_dbterm(&tb->common, p, mpi.mp, 0, - &(*current)->dbterm, NULL, 0); - if (is_value(match_res) && - is_value(key = db_getkey(tb->common.keypos, match_res)) && - eq(key, GETKEY(tb, (*current)->dbterm.tpl))) - { - // we need to make sure we don't end up with duplicate objects; - // it's quite inefficient - int object_exists = 0; - for (iter = initial; *iter != NULL; iter = &((*iter)->next)) - if (((*iter)->hvalue != INVALID_HASH) - && (*iter != *current) - && db_eq(&tb->common, match_res, &(*iter)->dbterm)) - { - object_exists = 1; - break; - } - - if (!object_exists) { - next = (*current)->next; - hval = (*current)->hvalue; - new = replace_dbterm(tb, *current, match_res); - new->next = next; - new->hvalue = hval; - *current = new; - ++got; - } - } - --num_left; - current = &((*current)->next); - } else { match_res = db_match_dbterm(&tb->common, p, mpi.mp, 0, &(*current)->dbterm, NULL, 0); @@ -2178,7 +2146,7 @@ static int db_select_replace_continue_hash(Process *p, { DbTableHash *tb = &tbl->hash; Uint slot_ix; - HashDbTerm **current = NULL, **initial = NULL, **iter = NULL; + HashDbTerm **current = NULL; HashDbTerm *new = NULL, *next = NULL; HashValue hval = INVALID_HASH; Eterm key; @@ -2213,7 +2181,6 @@ static int db_select_replace_continue_hash(Process *p, goto done; } current = &BUCKET(tb,slot_ix); - initial = current; for(;;) { if ((*current) == NULL) { @@ -2225,43 +2192,10 @@ static int db_select_replace_continue_hash(Process *p, goto trap; } current = &BUCKET(tb,slot_ix); - initial = current; } else if ((*current)->hvalue == INVALID_HASH) { current = &((*current)->next); } - else if (tb->common.status & DB_BAG) { - match_res = db_match_dbterm(&tb->common, p, mp, 0, - &(*current)->dbterm, NULL, 0); - if (is_value(match_res) && - is_value(key = db_getkey(tb->common.keypos, match_res)) && - eq(key, GETKEY(tb, (*current)->dbterm.tpl))) - { - // we need to make sure we don't end up with duplicate objects; - // it's quite inefficient - int object_exists = 0; - for (iter = initial; *iter != NULL; iter = &((*iter)->next)) - if (((*iter)->hvalue != INVALID_HASH) - && (*iter != *current) - && db_eq(&tb->common, match_res, &(*iter)->dbterm)) - { - object_exists = 1; - break; - } - - if (!object_exists) { - next = (*current)->next; - hval = (*current)->hvalue; - new = replace_dbterm(tb, *current, match_res); - new->next = next; - new->hvalue = hval; - *current = new; - ++got; - } - } - --num_left; - current = &((*current)->next); - } else { match_res = db_match_dbterm(&tb->common, p, mp, 0, &(*current)->dbterm, NULL, 0); -- cgit v1.2.3