benn/lumiera_
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Commands: refactor integration into SessionCommandService (#1089)

Browse source 
It seems more adequate to push the somewhat intricate mechanics
for the "fall back" onto generic commands down into the implementation
level of CommandInstanceManager. The point is, we know the standard
usage situation is to rely on the instance manager, and thus we want
to avoid redundant table lookups, only to support the rare case of
fallback to global commands. The latter is currently used only from
unit-tests, but might in future also be used by scripts.

Due to thread safety considerations, I have refrained from handing
out a direct reference to the command token sitting in the registry,
even while not doing so incurs a small runtime penalty (accessing
the shared ref-count for creating a copy of the smart-handle).
This is the typical situation where you'd be tempted to sacrifice
sanity for the sake of an imaginary performance benefit, which
in fact is dwarfed by all the machinery of UI-Bus and argument
passing via GenNode.
This commit is contained in:
Fischlurch 2017-04-09 19:11:40 +02:00
parent 45f86e42e4
commit aecef2a8f4
10 changed files with 192 additions and 68 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
src/include/session-command-facade.h
View file

@ -73,11 +73,22 @@ namespace control {
* qualify and elaborate the action, before it can be actually triggered.
* Commands then go through a queue to be invoked one by one.
*
* This is a layer separation facade interface. Clients should use
* the embedded #facade factory, which yields a proxy to route any
* The service exposed through this facade offers dedicated support for
* the _standard command cycle_, as is typically performed from the UI.
* Such a usage cycle starts with ["opening"](#cycle) a local anonymous
* clone copy from the global command definition, which is then used
* in further calls to be outfitted with actual arguments and finally
* to be handed over to the dispatcher for execution.
* @warning this standard command cycle is intended for single-threaded
* use from the UI. It is not threadsafe. To the contrary, all
* operations with globally registered commands are threadsafe.
*
* @remark This is a layer separation facade interface. Clients should
* use the embedded #facade factory, which yields a proxy to route any
* calls through the lumieraorg_SessionCommand interface
* @throws lumiera::error::State when interface is not opened
* @see [Command system](command.hpp)
* @see SessionCommandFunction_test
*/
class SessionCommand
{

5
src/proc/control/command-instance-manager.hpp
View file

@ -58,6 +58,7 @@
#define CONTROL_COMMAND_INSTANCE_MANAGER_H
#include "lib/error.hpp"
#include "proc/control/command.hpp"
#include "proc/control/command-dispatch.hpp"
#include "lib/symbol.hpp"
@ -91,6 +92,7 @@ namespace control {
* handle, the enqueued instance will stay alive until execution and then go out of scope. But, after
* #dispatch, it is no longer accessible from the CommandInstanceManger, and while it is still waiting
* in the execution queue, the next instance for the same invocationID might already be opened.
* @warning CommandInstanceManager is *not threadsafe*
*/
class CommandInstanceManager
: boost::noncopyable
@ -103,8 +105,7 @@ namespace control {
~CommandInstanceManager();
Symbol newInstance (Symbol prototypeID, string const& invocationID);
Command& getInstance(Symbol instanceID);
Command& maybeGetInstance (Symbol instanceID);
Command getInstance(Symbol instanceID);
void dispatch (Symbol instanceID);
bool contains (Symbol instanceID) const;

7
src/proc/control/command-registry.hpp
View file

@ -188,7 +188,12 @@ namespace control {
/** query the command index by ID
* @return the registered command,
* or an "invalid" token */
* or an "invalid" token
* @remark this function deliberately returns by-value.
* Returning a reference into the global CommandRegistry
* would be dangerous under concurrent access, since the
* lock is only acquired within this function's body.
*/
Command
queryIndex (Symbol cmdID)
{

53
src/proc/control/command-setup.cpp
View file

@ -38,6 +38,7 @@
#include "lib/error.hpp"
#include "include/logging.h"
#include "include/lifecycle.h"
#include "proc/control/command.hpp"
#include "proc/control/command-setup.hpp"
#include "proc/control/command-instance-manager.hpp"
#include "proc/control/command-def.hpp"
@ -192,40 +193,48 @@ namespace control {
}
/** @return handle to the currently "opened" instance with the given ID */
Command&
/** access the currently "opened" instance with the given instanceID
* @param instanceID ID as returned from #newInstance, or a global commandID
* @return instance handle of handle of a global command as fallback
* @note when given a commandID, which is not known as (decorated) instanceID
* within our local registration table, just the globally registered
* Command instance is returned.
* @remark deliberately this function returns by-value. Returning a reference
* into the global command registry would be dangerous under concurrency.
* @throw error::Invalid when the given cmdID unknown both locally and globally.
* @throw error::Logic when accessing an instance that _was_ known but is
* currently no longer "open" (already dispatched command instance)
*/
Command
CommandInstanceManager::getInstance (Symbol instanceID)
{
Command& instance = table_[instanceID];
if (not instance)
throw error::Invalid (_Fmt{"Command instance '%s' is not (yet/anymore) active"}
% instanceID
, LUMIERA_ERROR_INVALID_COMMAND);
return instance;
}
/** try to retrieve an currently active instance, but tolerate unknown IDs */
Command&
CommandInstanceManager::maybeGetInstance (Symbol instanceID)
{
static Command NOT_FOUND;
auto entry = table_.find(instanceID);
if (entry == table_.end())
return NOT_FOUND;
return Command::get(instanceID);
if (not entry->second)
throw error::Invalid (_Fmt{"Command instance '%s' is not (yet/anymore) active"}
% instanceID
, LUMIERA_ERROR_INVALID_COMMAND);
throw error::Logic (_Fmt{"Command instance '%s' is not (yet/anymore) active"}
% instanceID
, error::LUMIERA_ERROR_LIFECYCLE);
return entry->second;
}
/** hand over the designated command instance to the dispatcher installed on construction */
/** hand over the designated command instance to the dispatcher installed
* on construction. Either the given ID corresponds to a previously "opened"
* local instance (known only to this instance manager). In this case, the
* instance will really be _moved_ over into the dispatcher, which also means
* this instance is no longer "open" for parametrisation. In the other case
* we try to retrieve the given ID from the global CommandRegistry and
* dispatch it, _without removing_ it from the global registry of course.
*/
void
CommandInstanceManager::dispatch (Symbol instanceID)
{
Command& instance = table_[instanceID];
auto entry = table_.find(instanceID);
Command fallback;
Command& instance{entry == table_.end()? fallback = Command::get(instanceID)
: entry->second
};
if (not instance)
throw error::Logic (_Fmt{"attempt to dispatch command instance '%s' "
"without creating a new instance from prototype beforehand"}

3
src/proc/control/command.cpp
View file

@ -119,6 +119,9 @@ namespace control {
/** Access existing command for use.
* @throw error::Invalid if command not
* registered or incompletely defined.
* @remark this function deliberately returns by-value.
* Returning a reference into the global CommandRegistry
* would be dangerous under concurrent access.
*/
Command
Command::get (Symbol cmdID)

44
src/proc/control/session-command-service.cpp
View file

@ -55,25 +55,6 @@ namespace control {
using util::cStr;
/**
* @internal access the command instance for further processing.
* The standard use case is to get an anonymous command instance,
* which was previously opened within the CommandInstanceManager.
* But for exceptional cases, we also allow to access a global
* command instance by name.
* @param cmdID either the instanceID or the global cmdID
* @throw error::Invalid when no suitable command definition exists
*/
Command
SessionCommandService::retrieveCommand (Symbol cmdID)
{
Command cmdFound = instanceManager_.maybeGetInstance (cmdID);
return cmdFound? cmdFound
: Command::get(cmdID);
}
Symbol
SessionCommandService::cycle (Symbol cmdID, string const& invocationID)
{
@ -81,20 +62,35 @@ namespace control {
}
/**
* @param cmdID either the instanceID or the global cmdID
* @throw error::Invalid when no suitable command definition exists
* @throw error::Logic when the instance is already dispatched
* @note this function automatically _falls back_ on a global
* Command definition, in case the given ID is not known
* as a local command instance. This allows to use the
* SessionCommand service without explicit instantiation
*/
void
SessionCommandService::bindArg (Symbol cmdID, Rec const& argSeq)
{
retrieveCommand(cmdID).bindArg(argSeq);
instanceManager_.getInstance(cmdID)
.bindArg(argSeq);
}
/**
* @param cmdID either the instanceID or the global cmdID
* @throw error::Invalid when no suitable command definition exists
* @throw error::State when the command's arguments are not bound
* @throw error::Logic when the instance is already dispatched
* @note similar to #bindArg, this function _falls back_ on a global
* Command definition, in case the given ID is not known locally.
*/
void
SessionCommandService::invoke (Symbol cmdID)
{
if (instanceManager_.contains (cmdID))
instanceManager_.dispatch(cmdID);
else
dispatcher_.enqueue (Command::get(cmdID));
instanceManager_.dispatch(cmdID);
}

10
src/proc/control/session-command-service.hpp
View file

@ -31,7 +31,8 @@
** This service is the implementation of a layer separation facade interface. Clients should use
** proc::control::SessionCommand::facade to access this service. This header defines the interface
** used to _provide_ this service, not to access it.
**
**
** @see session-command-facade.h
** @see facade.hpp subsystems for the Proc-Layer
** @see guifacade.cpp starting this service
*/
@ -69,6 +70,11 @@ namespace control {
*
* This service is backed by implementation facilities embedded within
* the ProcDispatcher, exposed through the CommandDispatch interface.
* Additionally, it operates a CommandInstanceManager to allow the
* creation of local instances "opened" for argument binding.
* In fact, this is the standard "command cycle" and the
* intended usage pattern.
* @warning local command instances are not threadsafe
* @see DispatcherLoop
*/
class SessionCommandService
@ -78,8 +84,6 @@ namespace control {
CommandDispatch& dispatcher_;
CommandInstanceManager instanceManager_;
Command retrieveCommand (Symbol);
/* === Implementation of the Facade Interface === */

2
tests/12metaprogramming.tests
View file

@ -560,7 +560,7 @@ TEST "tuple initialisation from GenNode" TupleRecordInit_test <<END
out: Rec...GenNode.+DataCap.«string».lalü, GenNode.+DataCap.«int».42
out: Rec...GenNode.+DataCap.«string».lalü, GenNode.+DataCap.«string».lala, GenNode.+DataCap.«int».12, GenNode.+DataCap.«int».34, GenNode.+DataCap.«double».5.6, GenNode.+DataCap.«Time».0:09:08.007
out-lit: «tuple<string, int>»──(lalü,42)
out-lit: «tuple<idi::EntryID<long>, string, int, long, double, Duration>»──(ID<long>-lal,lala,12,34,5.6,≺548s7ms≻)
out-lit: «tuple<idi::EntryID<long>, Symbol, int, long, double, Duration>»──(ID<long>-lal,lala,12,34,5.6,≺548s7ms≻)
return: 0
END

34
tests/core/proc/control/command-instance-manager-test.cpp
View file

@ -114,6 +114,7 @@ namespace test {
verify_instanceIdentity();
verify_duplicates();
verify_lifecycle();
verify_fallback();
}
@ -135,7 +136,7 @@ namespace test {
iManager.dispatch (instanceID);
CHECK (fixture.contains (cmd));
CHECK (not iManager.contains (instanceID));
VERIFY_ERROR (INVALID_COMMAND, iManager.getInstance (instanceID));
VERIFY_ERROR (LIFECYCLE, iManager.getInstance (instanceID));
}
@ -293,9 +294,10 @@ namespace test {
Fixture fixture;
CommandInstanceManager iManager{fixture};
// a manually constructed ID is unknown of course
Symbol instanceID{COMMAND_PROTOTYPE, INVOCATION_ID};
VERIFY_ERROR (INVALID_COMMAND, iManager.getInstance (instanceID));
VERIFY_ERROR (LIFECYCLE, iManager.dispatch (instanceID));
VERIFY_ERROR (INVALID_COMMAND, iManager.dispatch (instanceID));
Symbol i2 = iManager.newInstance (COMMAND_PROTOTYPE, INVOCATION_ID);
CHECK (i2 == instanceID);
@ -313,9 +315,35 @@ namespace test {
iManager.dispatch (instanceID);
CHECK (not iManager.contains (instanceID));
VERIFY_ERROR (INVALID_COMMAND, iManager.getInstance (instanceID));
VERIFY_ERROR (LIFECYCLE, iManager.getInstance (instanceID));
VERIFY_ERROR (LIFECYCLE, iManager.dispatch (instanceID));
CHECK (instanceID == iManager.newInstance (COMMAND_PROTOTYPE, INVOCATION_ID));
}
/** @test the instance manager automatically falls back on globally registered commands,
* when the given ID is not and was not known locally */
void
verify_fallback()
{
Fixture fixture;
CommandInstanceManager iManager{fixture};
CHECK (not iManager.contains (COMMAND_PROTOTYPE));
Command cmd = iManager.getInstance (COMMAND_PROTOTYPE);
CHECK (cmd.isValid());
CHECK (not cmd.isAnonymous());
CHECK (cmd == Command::get(COMMAND_PROTOTYPE));
CHECK (cmd == Command{COMMAND_PROTOTYPE});
cmd.bind(-12);
CHECK (cmd.canExec());
CHECK (not fixture.contains(cmd));
iManager.dispatch (COMMAND_PROTOTYPE);
CHECK (fixture.contains(cmd));
}
};

87
wiki/thinkPad.ichthyo.mm
View file

@ -11791,10 +11791,76 @@
</node>
</node>
</node>
<node CREATED="1491753027936" HGAP="35" ID="ID_118278209" MODIFIED="1491757400403" TEXT="Refactoring" VSHIFT="2">
<icon BUILTIN="button_ok"/>
<node CREATED="1491753036895" ID="ID_1029946389" MODIFIED="1491753047201" TEXT="fall-back auf Command integrieren"/>
<node CREATED="1491753645531" ID="ID_597184850" MODIFIED="1491753650607" TEXT="maybeGet entfernen"/>
<node CREATED="1491753047981" ID="ID_1220888974" MODIFIED="1491753330643" TEXT="Referenz rausgeben">
<icon BUILTIN="button_cancel"/>
<node CREATED="1491753058620" ID="ID_1772710050" MODIFIED="1491753078866" TEXT="nicht thradsafe">
<icon BUILTIN="messagebox_warning"/>
</node>
<node CREATED="1491753170924" ID="ID_1947747818" MODIFIED="1491753316996" TEXT="Command-Zugriff ist by value">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
...aus gutem Grund
</p>
<p>
(kann mich erinnern, da&#223; ich mir das &#252;berlegt hatte).
</p>
<p>
Sofern Definitionen wirklich concurrent ge&#228;ndert oder gel&#246;scht werden,
</p>
<p>
k&#246;nnte es sein, da&#223; jemand auf einer stale reference arbeitet,
</p>
<p>
denn das Lock sch&#252;tzt nur den Aufruf innerhalb der CommandRegistry.
</p>
<p>
Sicher ist der Zugriff nur, wenn im Schutzbereich dieses Locks ein
</p>
<p>
neues Command-Objekt kopiert wird. Was allerdings den RefCount erh&#246;ht.
</p>
</body>
</html>
</richcontent>
</node>
<node CREATED="1491753132377" ID="ID_1441469124" MODIFIED="1491753156451" TEXT="m&#xfc;&#xdf;te daf&#xfc;r direkt auf die CommandRegistry zugreifen">
<icon BUILTIN="idea"/>
</node>
</node>
<node CREATED="1491753332318" ID="ID_889636506" MODIFIED="1491757384745" TEXT="nein: Wert rausgeben">
<linktarget COLOR="#3e657d" DESTINATION="ID_889636506" ENDARROW="Default" ENDINCLINATION="21;223;" ID="Arrow_ID_1428975472" SOURCE="ID_1707474386" STARTARROW="None" STARTINCLINATION="-541;0;"/>
<icon BUILTIN="button_ok"/>
<node CREATED="1491753337438" ID="ID_1228083262" MODIFIED="1491753342636" TEXT="KISS">
<icon BUILTIN="yes"/>
</node>
<node CREATED="1491753354116" ID="ID_1070880433" MODIFIED="1491753389816" TEXT="Bus + CommandHandler + GenNode">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
aber sich mit einem Refcount verr&#252;ckt machen.....
</p>
</body>
</html>
</richcontent>
<icon BUILTIN="smiley-oh"/>
</node>
</node>
</node>
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1491656361886" ID="ID_1269299900" MODIFIED="1491656365558" TEXT="Einbindung">
<icon BUILTIN="flag-yellow"/>
<node CREATED="1491659172438" ID="ID_1103739498" MODIFIED="1491694399272" TEXT="in SessionCommandService">
<icon BUILTIN="pencil"/>
<node COLOR="#338800" CREATED="1491659172438" HGAP="34" ID="ID_1103739498" MODIFIED="1491757376165" TEXT="in SessionCommandService" VSHIFT="-3">
<icon BUILTIN="button_ok"/>
<node CREATED="1491659191155" ID="ID_586991718" MODIFIED="1491659216131" TEXT="Nutzung des Instance-Managers ist optional"/>
<node CREATED="1491659194283" ID="ID_1169808852" MODIFIED="1491659203949" TEXT="direkter Zugriff auf Commands bleibt offen"/>
<node CREATED="1491659224630" ID="ID_837055372" MODIFIED="1491659238608" TEXT="Entscheidung anhand der Command-ID"/>
@ -11845,24 +11911,25 @@
<icon BUILTIN="stop-sign"/>
<node CREATED="1491702872058" ID="ID_487194706" MODIFIED="1491702875613" TEXT="keine gute Idee"/>
<node CREATED="1491702876137" ID="ID_141674543" MODIFIED="1491702880476" TEXT="doppelt das Interface"/>
<node CREATED="1491702881160" ID="ID_1657270200" MODIFIED="1491702949548" TEXT="Taschenspielerei">
<arrowlink COLOR="#a71f6e" DESTINATION="ID_1776414678" ENDARROW="Default" ENDINCLINATION="-59;-44;" ID="Arrow_ID_559634306" STARTARROW="None" STARTINCLINATION="126;32;"/>
<node CREATED="1491702881160" ID="ID_1657270200" MODIFIED="1491753472968" TEXT="Taschenspielerei">
<arrowlink COLOR="#a71f6e" DESTINATION="ID_1776414678" ENDARROW="Default" ENDINCLINATION="-8;-62;" ID="Arrow_ID_559634306" STARTARROW="None" STARTINCLINATION="-86;2;"/>
<icon BUILTIN="smily_bad"/>
</node>
</node>
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1491702805203" HGAP="26" ID="ID_1776414678" MODIFIED="1491702949548" TEXT="Performance / Standard-Fall" VSHIFT="22">
<linktarget COLOR="#a71f6e" DESTINATION="ID_1776414678" ENDARROW="Default" ENDINCLINATION="-59;-44;" ID="Arrow_ID_559634306" SOURCE="ID_1657270200" STARTARROW="None" STARTINCLINATION="126;32;"/>
<icon BUILTIN="flag-yellow"/>
<node CREATED="1491702805203" HGAP="26" ID="ID_1776414678" MODIFIED="1491757370278" TEXT="Performance / Standard-Fall" VSHIFT="22">
<linktarget COLOR="#a71f6e" DESTINATION="ID_1776414678" ENDARROW="Default" ENDINCLINATION="-8;-62;" ID="Arrow_ID_559634306" SOURCE="ID_1657270200" STARTARROW="None" STARTINCLINATION="-86;2;"/>
<icon BUILTIN="button_ok"/>
<node CREATED="1491702822568" ID="ID_1028353876" MODIFIED="1491702830531" TEXT="wir suchen stets zuerst eine Instanz"/>
<node CREATED="1491702831023" ID="ID_184396015" MODIFIED="1491702840810" TEXT="wir fallen auf die globale Registry zur&#xfc;ck"/>
<node CREATED="1491702841453" ID="ID_1707474386" MODIFIED="1491702856185" TEXT="das kann der InstanceManager am Besten selber!">
<node CREATED="1491702841453" ID="ID_1707474386" MODIFIED="1491757384744" TEXT="das kann der InstanceManager am Besten selber!">
<arrowlink COLOR="#3e657d" DESTINATION="ID_889636506" ENDARROW="Default" ENDINCLINATION="21;223;" ID="Arrow_ID_1428975472" STARTARROW="None" STARTINCLINATION="-541;0;"/>
<icon BUILTIN="idea"/>
</node>
</node>
</node>
<node CREATED="1491692398913" ID="ID_1349137025" MODIFIED="1491692404050" TEXT="in UI-Bus">
<node CREATED="1491692398913" HGAP="27" ID="ID_1349137025" MODIFIED="1491753610374" TEXT="in UI-Bus" VSHIFT="6">
<icon BUILTIN="help"/>
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1491692419958" ID="ID_411638678" MODIFIED="1491692425573" TEXT="#1058 consider expanding UI-Bus protocol for command cloning">
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1491692419958" ID="ID_411638678" MODIFIED="1491753600586" TEXT="#1058 consider expanding UI-Bus protocol for command cloning">
<icon BUILTIN="flag-yellow"/>
</node>
<node CREATED="1491692431021" ID="ID_1526880552" MODIFIED="1491692436375" TEXT="wollen wir das?"/>