From 44db4760a143fdefc1e61473a2985a805a1e801f Mon Sep 17 00:00:00 2001 From: theoreticalbts Date: Wed, 9 Dec 2015 17:22:38 -0500 Subject: [PATCH 1/2] undo_database.cpp: Refactor undo_database::merge() code This commit does not change semantics at all, it just changes the layout of the code and adds comments. --- libraries/db/undo_database.cpp | 86 +++++++++++++++++++++++++++++++--- 1 file changed, 80 insertions(+), 6 deletions(-) diff --git a/libraries/db/undo_database.cpp b/libraries/db/undo_database.cpp index e461c553..e28aa3bc 100644 --- a/libraries/db/undo_database.cpp +++ b/libraries/db/undo_database.cpp @@ -127,25 +127,99 @@ void undo_database::merge() FC_ASSERT( _stack.size() >=2 ); auto& state = _stack.back(); auto& prev_state = _stack[_stack.size()-2]; + + // An object's relationship to a state can be: + // in new_ids : new + // in old_values (was=X) : upd(was=X) + // in removed (was=X) : del(was=X) + // not in any of above : nop + // + // When merging A=prev_state and B=state we have a 4x4 matrix of all possibilities: + // + // |--------------------- B ----------------------| + // + // +------------+------------+------------+------------+ + // | new | upd(was=Y) | del(was=Y) | nop | + // +------------+------------+------------+------------+------------+ + // / | new | N/A | new A| nop C| new A| + // | +------------+------------+------------+------------+------------+ + // | | upd(was=X) | N/A | upd(was=X)A| del(was=X)C| upd(was=X)A| + // A +------------+------------+------------+------------+------------+ + // | | del(was=X) | N/A | N/A | N/A | del(was=X)A| + // | +------------+------------+------------+------------+------------+ + // \ | nop | new B| upd(was=Y)B| del(was=Y)B| nop AB| + // +------------+------------+------------+------------+------------+ + // + // Each entry was composed by labelling what should occur in the given case. + // + // Type A means the composition of states contains the same entry as the first of the two merged states for that object. + // Type B means the composition of states contains the same entry as the second of the two merged states for that object. + // Type C means the composition of states contains an entry different from either of the merged states for that object. + // Type N/A means the composition of states violates causal timing. + // Type AB means both type A and type B simultaneously. + // + // The merge() operation is defined as modifying prev_state in-place to be the state object which represents the composition of + // state A and B. + // + // Type A (and AB) can be implemented as a no-op; prev_state already contains the correct value for the merged state. + // Type B (and AB) can be implemented by copying from state to prev_state. + // Type C needs special case-by-case logic. + // Type N/A can be ignored or assert(false) as it can only occur if prev_state and state have illegal values + // (a serious logic error which should never happen). + // + + // We can only be outside type A/AB (the nop path) if B is not nop, so it suffices to iterate through B's three containers. + + // *+upd for( auto& obj : state.old_values ) { if( prev_state.new_ids.find(obj.second->id) != prev_state.new_ids.end() ) + { + // new+upd -> new, type A continue; - if( prev_state.old_values.find(obj.second->id) == prev_state.old_values.end() ) - prev_state.old_values[obj.second->id] = std::move(obj.second); + } + if( prev_state.old_values.find(obj.second->id) != prev_state.old_values.end() ) + { + // upd(was=X) + upd(was=Y) -> upd(was=X), type A + continue; + } + // nop+upd(was=Y) -> upd(was=Y), type B + prev_state.old_values[obj.second->id] = std::move(obj.second); } + + // *+new, but we assume the N/A cases don't happen, leaving type B nop+new -> new for( auto id : state.new_ids ) prev_state.new_ids.insert(id); + + // old_index_next_ids can only be updated, iterate over *+upd cases for( auto& item : state.old_index_next_ids ) { if( prev_state.old_index_next_ids.find( item.first ) == prev_state.old_index_next_ids.end() ) + { + // nop+upd(was=Y) -> upd(was=Y), type B prev_state.old_index_next_ids[item.first] = item.second; - } - for( auto& obj : state.removed ) - if( prev_state.new_ids.find(obj.second->id) == prev_state.new_ids.end() ) - prev_state.removed[obj.second->id] = std::move(obj.second); + continue; + } else + { + // upd(was=X)+upd(was=Y) -> upd(was=X), type A + // type A implementation is a no-op, as discussed above, so there is no code here + continue; + } + } + + // *+del + for( auto& obj : state.removed ) + { + if( prev_state.new_ids.find(obj.second->id) != prev_state.new_ids.end() ) + { + // new + del -> nop (type C) prev_state.new_ids.erase(obj.second->id); + continue; + } + // nop + del(was=Y) -> del(was=Y) + prev_state.removed[obj.second->id] = std::move(obj.second); + } _stack.pop_back(); --_active_sessions; } From c2943ee3bb4075dbfd88003fdcc0173e1c13a437 Mon Sep 17 00:00:00 2001 From: theoreticalbts Date: Wed, 9 Dec 2015 17:26:52 -0500 Subject: [PATCH 2/2] undo_database.cpp: Handle unimplemented upd+del case --- libraries/db/undo_database.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libraries/db/undo_database.cpp b/libraries/db/undo_database.cpp index e28aa3bc..86f6bb21 100644 --- a/libraries/db/undo_database.cpp +++ b/libraries/db/undo_database.cpp @@ -183,6 +183,8 @@ void undo_database::merge() // upd(was=X) + upd(was=Y) -> upd(was=X), type A continue; } + // del+upd -> N/A + assert( prev_state.removed.find(obj.second->id) == prev_state.removed.end() ); // nop+upd(was=Y) -> upd(was=Y), type B prev_state.old_values[obj.second->id] = std::move(obj.second); } @@ -217,6 +219,16 @@ void undo_database::merge() prev_state.new_ids.erase(obj.second->id); continue; } + auto it = prev_state.old_values.find(obj.second->id); + if( it != prev_state.old_values.end() ) + { + // upd(was=X) + del(was=Y) -> del(was=X) + prev_state.removed[obj.second->id] = std::move(it->second); + prev_state.old_values.erase(obj.second->id); + continue; + } + // del + del -> N/A + assert( prev_state.removed.find( obj.second->id ) == prev_state.removed.end() ); // nop + del(was=Y) -> del(was=Y) prev_state.removed[obj.second->id] = std::move(obj.second); }