-
Notifications
You must be signed in to change notification settings - Fork 267
Conversation
# Conflicts: # core/Cargo.toml # core/src/agent/state.rs # core/src/lib.rs # core/src/state.rs
this actually does a round trip! time to start polishing.. |
core/src/agent/state.rs
Outdated
self.top_pair.clone() | ||
/// getter for the chain | ||
pub fn chain(&self) -> Chain { | ||
self.chain.clone() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe not do this. This can get out of hand when used overly. better return an immutable reference to self.chain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sphinxc0re ok i'll change it, i have some questions about this and mutability but they can wait until later
…st into 135-riker-one-sys
…r-one-sys # Conflicts: # core/src/instance.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fabulous. Big step forward.
impl AskSelf for ActorRef<Protocol> { | ||
fn block_on_ask(&self, message: Protocol) -> Protocol { | ||
let a = ask(&(*SYS), self, message); | ||
block_on(a).unwrap() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we make this a Result?
/// every action and the result of that action | ||
// @TODO this will blow up memory, implement as some kind of dropping/FIFO with a limit? | ||
// @see https://github.com/holochain/holochain-rust/issues/166 | ||
actions: HashMap<ActionWrapper, ActionResponse>, | ||
chain: Chain, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this much more that the state has a chain
than a top_pair
} | ||
|
||
#[test] | ||
/// show two things here: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice test!!
) | ||
|
||
(func | ||
(export "commit_dispatch") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think his will conflict with #268 so someone will have to do the merge...
0. the actor ref calls its own `ask` method, which builds a future using riker's `ask` | ||
0. the actor ref blocks on its internal future | ||
0. the referenced actor receives the `Commit` message and matches/destructures this into the entry | ||
0. the entry is passed to the `commit()` method of the inner table |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit concerned of all those commit()
s here which actually should be put()
s.
I've created ticket #274 as a follow up task to make these function names match the mental modal we have...
IMPORTANT: tests are failing CI because the docker box uses stable and we need nightly. test locally for review
fixes #135
fixes #137
this introduces the Riker actor system for #135
the problem
the state tree (redux style state) is great for small, well known items of data but struggles with:
(actual photo of generic traits spreading through our application state)
this solution: riker actors
http://riker.rs/ implementing the riker library for actors
the first implementation is for hash tables
riker core concepts:
Actor
that creates new actor instances and defines messagereceive
ActorRef<MyProtocol>
that cantell
messages to the actor instance it references via. the actor systemthe approach in this PR is to implement the
HashTable
trait for actor refs. the actor ref is passed an "inner table" that also implements the sameHashTable
trait at construction time. the actor ref becomes a standardised, transparent wrapper around the inner table implementation.this means that calling
table.commit()
andtable_actor_ref.commit()
do the same thingthe benefit of table actor refs:
Arc
reference counting, no locks, etc.implementation deets
the 1:1 API implementation between actors and their inner table is achieved by internally blocking on an
ask
from riker patterns - https://github.com/riker-rs/riker-patternsthe actor ref methods implementing
HashTable
send messages to itselfcalling
table_actor_ref.commit(entry)
looks like this:HashTableProtocol::Commit
message including the entryask
method, which builds a future using riker'sask
Commit
message and matches/destructures this into the entrycommit()
method of the inner tableHashTable
, does something with commit (e.g. MemTable inserts into a standard Rust, in-memoryHashMap
)commit
is inserted into aCommitResult
messageCommitResult
message is sent by the actor back to the actor ref's internal futureCommitResult
message is destructured by the actor ref so that the return ofcommit
satisfies theHashTable
trait implementationriker
ask
returns a future from the futures0.2.2
crate,table_actor.ask
callsblock_on
andunwrap
against this ask. both the block and the unwrap should be handled better in the future.limitations, tradeoffs
i wish that this were free
nightly rust
IIRC riker (or futures, or both) needs nightly rust (TODO: double check this, document why)
rust futures
the futures story for rust is "WIP"
await
is ambiguous after updated to nightly-2018-06-24 and after alexcrichton/futures-await#1060.2.x
,0.3.x
and the nightly compilereven so, futures look far more pragmatic than thread/channel juggling for many situations
for example, the observer/sensor/event-loop state model we implemented ad-hoc looks a lot like some of the future/poll/task system internals
dependency on riker
once/if we merge this, we're pretty much in bed with riker moving forward:
riker
ask
actor/future vs. futures pollrust has limited/awkward callback support so futures in rust (unlike basically every other language with futures) is poll based, and must be "driven" externally
the native future model is designed primarily to be composable on the level of
poll
. the whole thing only works because nestedpoll
calls in nested futures can bubble their results.the usefulness of futures comes in large part through the various abstractions for controlling how nested/parallel poll results are are called, merged, blocked on, etc.
riker on the other hand is all about independent actors sending and receiving messages asynchronously. the need for futures to make this work is equal parts implementation detail and "adapter" for the broader rust ecosystem. i get the feeling that if there was a viable non-futures approach, then riker would use that instead.
for example, i couldn't figure out how to usefully nest
ask
futures across multiple actors like we could nestpoll
calls in a vanilla future. the underlying futures task context (needed by poll) is hidden somewhere in the riker internals. nested blocking onask
is a compiler error.at this point i'm willing to chalk the friction up to my own inexperience with futures/riker and the relative immaturity of both libraries. it's certainly not clear that we have a hard requirement for nested actors... after all, i managed to find a way around it for the HashTable use-case.
Why not use...
Structs
We could fix the generic trait issues by:
new
method of the wrapper takes a<HT: HashTable>
and acts as a bufferthe problem (ignoring likely unknown size issues for the inner table) is that cloning the wrapper struct also clones the inner table. some inner implementations might be a stateless reference (e.g. a URL) and be safe to clone. Many will be stateful (e.g. MemTable) and so can't be cloned safely.
riker actor references are always stateless. the actor + actor system quarantines state for us, regardless of the inner implementation.
Actix
Actix looks great:
but has these limitations that looked like dealbreakers when i reviewed with the team:
changes
💥 💥 💥
💥 💥 💥
get
vs.get_entry
andget_pair
HashTable
to be actor ref friendlyHashTable
for actor refChain
to use theHashTable
actor ref instead of the inner implementationMemTable
for agent statefollowups
HashTable
setup and teardown? do we need setup/teardown for HashTable? #262