-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Redesign of typed-protocols #3
Conversation
7718880
to
41395a3
Compare
3a7bf92
to
ec3e9dc
Compare
92ceaab
to
5321dc0
Compare
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.
Just did a first pass, and made some high level comments. It would help if some commits got squashed or maybe re-ordered so it is easier to review next time!
typed-protocols-examples/src/Network/TypedProtocol/Driver/Simple.hs
Outdated
Show resolved
Hide resolved
typed-protocols-examples/src/Network/TypedProtocol/Driver/Simple.hs
Outdated
Show resolved
Hide resolved
typed-protocols-examples/src/Network/TypedProtocol/Driver/Simple.hs
Outdated
Show resolved
Hide resolved
-> (forall st'. Message ps st st' | ||
-> Peer ps pr pl Empty st' m a) | ||
-- ^ continuation | ||
-> Peer ps pr pl Empty st m a |
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.
Does it make sense to await for a non empty queue?
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.
Yes, this is useful for non-pipelined clients / servers. The queue is only filled when we are postponing transitions due to pipelining.
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.
My question is different. I am asking if it makes sense having a similar contructor for non-empty queues.
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.
When pipelining queue is non empty the only way to take something is to use Collect
or CollectSTM
which are pulling transitions from the left side of the queue - correctness depends on this.
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.
could we convert a non-empty pipelined protocol to a non-pipelined protocol by waiting for all the transitions and then proceeding normally?
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.
Yes we do such a conversion, check forgetPipelined
.
typed-protocols-examples/test/Network/TypedProtocol/ReqResp/Tests.hs
Outdated
Show resolved
Hide resolved
5321dc0
to
f6b0208
Compare
type ReflRelativeAgency :: Agency -> RelativeAgency -> RelativeAgency -> Type | ||
data ReflRelativeAgency a r r' where | ||
ReflClientAgency :: ReflRelativeAgency ClientAgency r r | ||
ReflServerAgency :: ReflRelativeAgency ServerAgency r r | ||
ReflNobodyAgency :: ReflRelativeAgency NobodyAgency r r |
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.
@dcoutts the role of this type is indeed confusing. Would it help if we provide type aliases for how we want to use this type:
-- | Type of the proof that we have the agency.
--
-- 'ReflClientAgency' has this type only iff `'StateAgency' st ~ 'ClientAgency'`
-- and `pr ~ 'AsClient'`.
--
-- 'ReflServerAgency' has this type only iff `'StateAgency' st ~ 'ServerAgency'`
-- and `pr ~ 'AsServer'`
--
type WeHaveAgencyProof :: PeerRole -> ps -> Type
type WeHaveAgencyProof pr st = ReflRelativeAgency
(StateAgency st)
WeHaveAgency
(Relative pr (StateAgency st))
-- | Type of the proof that the remote side has the agency.
--
-- 'ReflClientAgency' has this type only iff `'StateAgency' st ~ 'ClientAgency'`
-- and `pr ~ 'AsServer'`.
--
-- 'ReflServerAgency' has this type only iff `'StateAgency' st ~ 'ServerAgency'`
-- and `pr ~ 'AsClient'`
--
type TheyHaveAgencyProof :: PeerRole -> ps -> Type
type TheyHaveAgencyProof pr st = ReflRelativeAgency
(StateAgency st)
TheyHaveAgency
(Relative pr (StateAgency st))
-- | Type of the proof that nobody has agency in this state.
--
-- Only 'ReflNobodyAgency' can fulfil the proof obligation.
--
type NobodyHasAgencyProof :: PeerRole -> ps -> Type
type NobodyHasAgencyProof pr st = ReflRelativeAgency (StateAgency st)
NobodyHasAgency
(Relative pr (StateAgency st))
Pattern matches on ReflRelativeAgency
bring two things into scope:
- what is the
Agency
(the absolute one) - and depending on the type
WeHaveAgencyProof
,TheyHaveAgencyProof
orNobodyHasAgencyProof
a proof that the relative agency is the right one for the peer role & state.
We used to use:
data PeerHasAgency (pr :: PeerRole) (st :: ps) where
ClientAgency :: !(ClientHasAgency st) -> PeerHasAgency AsClient st
ServerAgency :: !(ServerHasAgency st) -> PeerHasAgency AsServer st
type WeHaveAgency (pr :: PeerRole) st = PeerHasAgency pr st
type TheyHaveAgency (pr :: PeerRole) st = PeerHasAgency (FlipAgency pr) st
Pattern matching on PeerHasAgency
brings into scope the peer role and the state type. The difficulty with that is then partition proofs has to be written per Protocol
instance, while proofs for ReflRelativeAgency
can be done once for all protocols. This becomes possible because Refl
is an equivalence relation. For example in the context of excludeLemma_ClientAndServerHaveAgency
we have ra ~ (Relative pr a)
and ra ~ (Relative (FlipAgency pr) a)
(both provided by the RelfRelativeAgency
), we can infer that Relative pr a ~ Relative (FlipAgency pr) a
, from definitions of Relative
and FlipAgency
it's easy to see why this can only happen when StateAgecy st ~ NobodyAgecy
. In this proof we don't need to mention what is the state.
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.
If we want to use WeHaveAgencyProof
(or a similar name like WeHaveAgencyType
), then I would avoid using WeHaveAgency
for the RelativeAgency
type: too many things would be called this way.
PeerHasAgency pr st | ||
decode :: forall (st :: ps). | ||
ActiveState st | ||
=> Sing st |
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.
@dcoutts we actually cannot use StateToken st
here. When I do I start to have problems to fill it when calling the decode
method of Codec
:
decode :: forall (st :: ps).
ActiveState st
=> StateToken st
-> m (DecodeStep bytes failure m (SomeMessage st))
src/Network/TypedProtocol/Codec.hs:338:17: error:
• Couldn't match type: Sing
with: StateToken
arising from a use of ‘stateToken’
both StateToken
type family and
stateToken :: (SingI st, Sing st ~ StateToken st) => StateToken st
stateToken = sing
are a bit artificial, it ought to always satisfy the constraint Sing st ~ StateToken st
.
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.
Why do you still require Sing st
if you already have StateToken
then?
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.
it seems I haven't explained it well :); I got the above error at call sites.
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.
Got it!
@dcoutts
Currently we pass the singleton using type class dictionary rather than an explicit argument, except when it is anyway needed by the library user (which is only in Note that one can always translate one into the other with The current design choice makes it easier to write a Some time ago I tried a different approach: to remove type family StateToken ...
class StateTokenI ... but then one would need to provide |
881bad3
to
b596c18
Compare
5fcf5a2
to
e808e5c
Compare
42ea29d
to
571afc6
Compare
Rebased on top of current |
Based on Okasaki's `BatchedQueue`s.
The simplest solution seem to add `StateAgency st ~ NobodyAgency` to the `Done` constructor. Which can be carried over to `TerminalStates`.
…oerces The 'IsLast' constraint allows to verify that after collecting all transitions the peer will continue in the right state.
`stateful` packages for consistency are also bumped to `0.2.0.0`.
* WeHaveAgencyProof * TheyHaveAgencyProof * NobodyHasAgencyProof todo: use them in `typed-protocols-stateful`
This allows us to not worry about the promoted type and kind to have the same name.
7c45ba0
to
cd44e0a
Compare
cd44e0a
to
fdfc6c5
Compare
Below there are to memory profiles of two versions of In both cases we use
Both cases were running with
|
Accumulated downloaded block sizes over time:
This is measured with |
HypothesisThe difference between the new design ( ValidationTo verify the hypothesis I run a 10000 -N2fib 25
fib 30
fib 33
fib 35
10000 -N2fib 25
fib 30
fib 33
fib 35
|
Possible ApproachesThere might be two possible approaches, with various pros & cons. Lazy deserialisation.If we provide length of messages (which would require to modify our codecs, or possibly make mux aware of message boundaries, then we could read data from the network, pass it to another thread which would deserialise it and act on it, e.g. add a block to Extension of
|
A major redesign of
typed-protocols
.Missing Todo Items:
tx-submission
mini-protocol)singletons
librarytyped-protocols-0.1