c80k / capnproto-dotnetcore

A Cap'n Proto implementation for .NET Standard and .NET Core
Other
145 stars 27 forks source link

Defining specs that expose existing C# types is boilerplate heavy #64

Open nothingmuch opened 2 years ago

nothingmuch commented 2 years ago

When faced with the task of implementing the generated interfaces in terms of pre-existing types that are not open to extension, additional boilerplate classes are required in order to facilitate the conversions. This is tedious and error prone.

A simpler way to solve this might be to add a flag to generate partial classes in order to allow the definition of conversion operators directly on the generated types.

Alternatively, given that READER and WRITER classes are already defined for each domain class, perhaps it would make sense to add two corresponding interfaces ITStructReader and ITStructWriter, following the concrete READER and WRITER, and then define an ITStruct interface for every struct type T which the struct type also satisfies.

When a struct occurs in an argument, instead of specifying the concrete domain class the generated code could require the ITStruct interface

For example:

struct Foo {
   foo @0 :Int32;
}

interface Bar {
  frobnicate @0 (foo :Foo) -> ();
}

that could result in something like:

public interface IFooReader { int Foo { get; } }
public interface IFoo: IFooReader { int Foo { set; } } // separate IFooWriter?
public struct Foo : IFoo
{
    public struct READER : IFooReader { ... }
    public struct WRITER: IFoo { ... }
    ...
}

so that the generated Bar interface could accept different ways of representing the Foo argument:

public interface IBar
{
    Task Frobnicate(IFoo foo, CancellationToken cancellationToken_ = default);
}

allowing a consumer of this interface can pass in an arbitrary type satisfying the interface.

If I'm not mistaken on the provider side of the interfaces, a similar conversion can be done depending on the consumer's preference for a concrete representation of the struct type:

public interface IBar<TFoo> : IBar
where TFoo : IFoo
{
    Task Frobnicate<TFoo>(TFoo foo, CancellationToken cancellationToken_ = default);

    // Satisfy the regular IBar by using an explicit conversion operator
    Task Frobnicate(IFooReader foo, CancellationToken cancellationToken_ = default) => Frobnicate<TFoo>(TFoo(foo), cancellationToken_);
}

so I think this would be backwards compatible.

c80k commented 2 years ago

Could you elaborate on your extension scenario - what types are pre-existing in your case? I have some difficulties following the outlined example. Is the goal to use Foo, Foo.READER, or Foo.WRITER interchangeably for argument foo? Or do you want to pass a custom implementation for foo?

nothingmuch commented 2 years ago

The goal is to minimize the writing of repetitive conversion code by hand, and to have a logical place for that conversion code to the extent that it is necessary. I also proposed two possible mechanisms that might be able to do that (not sure yet, although I do know for sure it's not a complete solution). Neither of these is a goal on its own (and I might be having an x y problem and missing the simple/obvious way of achieving what I want).

FWIW it seems like the following two solutions are not mutually exclusive, but perhaps the second approach despite being more complex might offer other advantages for avoiding or minimizing copying, so maybe it's worth considering even if the first solution seems acceptable.

context

I'm using NBitcoin which provides a number of types, in particular TxOut, TxIn which together form a Transaction.

There are also capnproto schemas corresponding to these types (all structs made of primitive types), and capabilities which use them, I need to address the sub-structure, so I have generated versions of objects containing exactly the same information, but with distinct types and under distinct namespaces.

A very simplified example:

struct Signature { raw :Data } # not a real type
struct Transaction { raw :Data } # ideally this should be a struct with sub-structure: { txIns :List(TxIn); txOuts :List(TxOut) } 
interface Signer {
    # this capability might be implemented in terms of specialized signing device
    sign @0 (unsignedTx :Transaction) -> (sig :Signature);
}

In my code I then have an interface:

interface MySign {
    Task<Signature> SignTransactionAsync(NBitcoin.Transaction unsignedTx, CancellationToken _)
}

and a client and a server implementation of each capnproto interface:

// not really records since they aren't value types, this is just for brevity
record SignerClient(IMySigner Signer) : Capnp.ISigner { ... }

record SignerServer(Capnp.ISigner Signer) : IMySIgner { ... }

To implement these interfaces, these only translate arguments of the above types, so e.g. a client that implements and which wraps a capnproto client side of the above Sign capability, and just converts NBitcoin.Transaction to Capnp.Transaction, and vice versa.

problem

The code to convert between these two types gets kind of unwieldly when there are several layers of nesting. For the above schema it's just new() { Data = tx.ToBytes() } but if actually doing this in terms of the sub-structure of Transaction (2 ints and two lists of structs, one of which has and additional sub-struct) means 2 functions for each type, with dependencies between these for nested types.

Unfortunately the most logical place to put these is in a new class to handle the conversions between these pairs of types and expose them as static methods, or to introduce a wrapper class for each pair e.g. MyTransactionWrapper, MyTxOutWrapper, etc etc, with conversions to and from Capnp.Transaction & NBitcoin.Transaction, Capnp.TxOut & NBitcoin.TxOut, etc.

possible solution A: partial all generated types

The simple approach would be to emit out the generated Capnp.Transaction type as a partial class. My understanding is that this is pretty standard for code generation.

It seems this could be toggled with schema annotations in a very backwards compatible way, either per schema (like namespace) or per type (like member annotations).

If that were the case then I believe (haven't verified yet, sorry) that could just implement an implicit conversion operator to NBitcoin.Transaction and an explicit constructor from one. Whenever I need an NBitcoin.Transaction (e.g. to IMySigner.Sign) I could pass in an Capnp.Transaction (they are value types so implicit conversion should be fine), and whenever I need a Capnp.Transaction and have an NBitcoin.Transaction I just need to wrap it in new(x) (and if that type were open for extension, I could implement an implicit conversion in that direction as well, but the only way of doing that is a parallel inheritance hierarchy so wrapping it seems cleaner).

possible solution B: generate more interfaces

If instead of converting, I could implement a hypothetical interface type Transaction.IREADER on a thin wrapper around NBitcoin.Transaction (MyTransactionWrapper), that would save an unnecessary 2nd copying step into the generated domain type, and simplify my code a little. This is a bit like implementing the deserializable interface by hand, but for a statically determined capnproto type.

Secondly, if I implemented a hypothetical IWRITER implement the server side interface for the Sign capability as ISign<MyTransactionWrapper> allowing the same generated code to construct this type instead of the generated struct type (since that is an upstream dependency for me I would have to settle with a wrapper), then when reading messages off the wire, MyTransactionWrapper could be used instead of the generated domain type and the generated code to read the data out of the segment and into the struct would write directly into the underlying NBitcoin.Transaction. This kind of conversion only requires one wrapper per domain type, and places the explicit conversion code in a slightly more logical place.

Copying a reader's data into a writer should be fairly straight forward, and in some cases it might make sense to short circuit and wire them directly (e.g. with promise pipelining a capability might receive an argument that is the result of invoking another, so no copying is necessary, and the concrete reader implementation for the argument side of the value could just proxy the reader interface directly to the underlying value in the fulfilled promise.

Right now disabling domain class generation inhibits creation of interface types for capabilities and the domain class wrappers, but with these interfaces for readers & writers, the capability interfaces could be generated even without the concrete domain classes, since they would not directly depend on their definition. This suggests the possibility of generating capability interfaces and concrete domain classes independently of each other, depending on whether or not the user wishes to build on the domain classes, or just use them as a temporary representation for data coming in and out of the wire. In this regard partial is entirely complementary to these hypothetical interfaces.

I hope that's clearer, if not I can share real code if you're interested, but it's very much PoC quality and kind of verbose/messy due to a variety of additional constraints.

c80k commented 2 years ago

Ok, I think I got your point.

Solution A should be pretty easy to realize. The code generator could even emit this by default since the additional partial keyword should not cause any harm. Another option, not requiring partial classes, could be using extension methods. E.g. you might implement some Capnp.Transaction.ToNBitcoin() and vice versa.

I still have to think about the feasibility of solution B. The decision to base RPC features entirely on domain classes was due to the following observation: READER structs / WRITER classes are easy to consume but cumbersome to provide. E.g. it is easy to extract data from a READER struct but difficult to construct it. Fortunately, you don't need to worry about that part because the library does it for you. A READER is just a small wrapper around a DeserializerState, which is also a struct. The DeserializerState provides low-level access to the underlying raw data. Hence, a hypothetical IREADER interface would be required to expose that DeserializerState.

For RPC scenarios, arguments passed between client and server must be both easy to read AND easy to construct because we have to consider both sides. This is why READER-like arguments are not feasible. WRITER-like arguments are also out of consideration because they are inherently linked to a data construction context. Hence, the conversion of READER to WRITER is not only a quite heavy operation, but RPC clients would have to fiddle with the MessageBuilder contexts.

So far, I don't think that switching to the READER/WRITER-style interfaces would solve your problem. We might see some performance improvements using the lower-level stuff, but this is a different topic.

What about option C: If NBitcoin and Capnp share some common formal data schema, you might implement a code generator that implements all the nevessary conversions?

nothingmuch commented 2 years ago

Solution A should be pretty easy to realize. The code generator could even emit this by default since the additional partial keyword should not cause any harm.

👍

If you're fine with partial I can open a pull request.

I think I've written too much already (sorry), so unless you find it interesting I propose to just forget the other idea now and when/if I feel I could actually use it. If the approach is helpful, then I could do a proof of concept quite easily (just write additional types by hand first, and then try to tweak the codegen to do it) and with actual working code it'll be easier to see if there's any merit to it, but right now I feel like I'm kind of wasting your time.


Another option, not requiring partial classes, could be using extension methods. E.g. you might implement some Capnp.Transaction.ToNBitcoin() and vice versa.

Yes, although that is a bit limited (no implicit operators, or implementing already existing interfaces)

I still have to think about the feasibility of solution B. The decision to base RPC features entirely on domain classes was due to the following observation: READER structs / WRITER classes are easy to consume but cumbersome to provide. E.g. it is easy to extract data from a READER struct but difficult to construct it. Fortunately, you don't need to worry about that part because the library does it for you. A READER is just a small wrapper around a DeserializerState, which is also a struct. The DeserializerState provides low-level access to the underlying raw data. Hence, a hypothetical IREADER interface would be required to expose that DeserializerState.

If only something like IFooReader as above is required, then I don't think that's true. If the interface is aable to provide all of the data for those types, that's equivalent, because you can read out enough information to fully reconstruct a state for a capnproto message consisting only of that object, but the interface should be narrow enough that it would not be possible to tell if it's backed by reading off capnproto formatted buffers or some other underlying representation.

For RPC scenarios, arguments passed between client and server must be both easy to read AND easy to construct because we have to consider both sides. This is why READER-like arguments are not feasible. WRITER-like arguments are also out of consideration because they are inherently linked to a data construction context. Hence, the conversion of READER to WRITER is not only a quite heavy operation, but RPC clients would have to fiddle with the MessageBuilder contexts.

I think that's a time/size tradeoff, it's not clear which is better a priori and depends on the implementation.

Right now with the domain classes I can implement the following, and in doing so I explicitly signal that I want copies of the capnproto data in the generated struct Foo:

class MyBarEager : Capnp.IBar
{
    Task Frobnicate(Foo foo) => doSomething(foo.Foo);
}

Ignoring for a moment the fact that in my example it only has a single int property, I pay a compute cost proportional to Foo's sizebut if I keep parts of it around the garbage collector can reclaim the parts I no longer need without me needing to do anything, so this is appropriate especially when splitting things up and sending them to multiple downstream functions.

If I only plan on reading a small part of that input, I could in principle write this instead:

class MyBarLazy : Capnp.IBar<Foo.Reader>
{
    Task Frobnicate(Foo.Reader foo) => doSomething(foo.Foo);
}

to indicate that I prefer the segment backed version and copy bits out myself, lazily constructing a view only into what I need, but I better let go of that Foo.Reader because keeping a pointer to it be holding on to a large chunk of memory.

The runtime has the information required to construct the concrete Foo.Reader with the DeserializerState.

Or I could indicate that I don't care and the runtime is free to choose:

class MyBarAgnostic : Capnp.IBar<IFooReader>
{
    Task Frobnicate(IFooReader foo) => doSomething(foo.Foo);
}

And at any time an IFooReader could also be trivially converted to a new Foo(IFooReader) to make a serializable copy regardless of whether that's another Foo,

Finally, If I also implement IFoo on some object, MyFoo:

class MyBarSpecific : Capnp.IBar<MyFoo>
{

}

class MyFoo : IFoo
{
    public int Foo { get; set; }
}

and because IFoo only requires int Foo { get; set } the runtime could create a new MyFoo without requiring any auxiliary data, and internally use the concrete Foo.Reader to copy over all the properties into MyFoo in much the same way as the generated Foo does.

So far, I don't think that switching to the READER/WRITER-style interfaces would solve your problem. We might see some performance improvements using the lower-level stuff, but this is a different topic.

In my particular situation the best I can do is subclass NBitcoin.Transaction etc, which I think is going to be kind of messy, so I'm a bit out of luck and I need to resort to a wrapper type or some other form of boilerplate either way.

But for classes I do control, of which there are a few as well, it would still be helpful to have more interoperability and polymorphism when implementing capability interfaces, that can reduce the friction when trying to package existing APIs as capnp specs, and to have finer grained control over how the generated structs are used and what representation/layout things have under the hood.

One example:

struct OutPoint {
    txid @0 :UInt256;
    vout @1 :UInt32;
}

struct Coin {
    outpoint @0 :OutPoint;
    txOut @1 :TxOut;
}

...
using Bitcoin = import "bitcoin.capnp";

...

interface SpendCapability {
    proveOwnership @0 (commitmentData :CommitmentData) -> (ownershipProof :OwnershipProof);
    sign @1 (unsignedTransaction :RawTransaction) -> (index :UInt32, witness :Bitcoin.WitScript);r
}

# spend capability implies exclusive control, release by disposing
struct SpendableCoin {
    coin @0 :Bitcoin.Coin;
    spendCapability @1 :SpendCapability;

    ... # and some more capabilities, some of which can fetch metadata
}

interface Wallet {
    getAvailableCoins @0 () -> (coins :List(SpendableCoin));
    generateSelfSpendScripts @1 (count :Int32) -> (scriptPubKeys :List(Bitcoin.Script));
}

In the implementation that I'm adapting SpendableCoin is realized by constructing it and setting the fields, but it's not really a value type (although the underlying Coin is), this struct is just a way of organizing the capabilities associated with a coin that results in slightly less boilerplate on the consumer side than a method that returns the Bitcoin.Coin information, which is always necessary...

Being able to think of SpendableCoin not just as a struct but also as a pair of interfaces that I can just implement in terms of other interfaces would give me more flexibility, so that the capnp side and the implementation side wouldn't need to correspond as closely.

What about option C: If NBitcoin and Capnp share some common formal data schema, you might implement a code generator that implements all the nevessary conversions?

That sounds like capnproto-dotnetcore ;-)

Seriously though the capnp schema is rich enough to describe the data, and with partial the codegen would be flexible enough to avoid the boilerplate, so in this regard if you're fine with that I could try to open a pull request and just implement that.

c80k commented 2 years ago

Feel free to go ahead with partial. Regarding the class organization It would make sense to have a clearer view on how IFooReader would look like, and how that compares to domain class Foo. Does Foo implement IFooReader? Does Reader implement IFooReader? As said, I don't see any easy way to unite Foo and Foo.READER in a common interface. Given a signature like Task Frobnicate(Foo.Reader foo), how should this look like on the caller side? Would we expect callers to provide a Foo.Reader? Or would we need to break the symmetry between callers and callees, effectively generating two different interfaces, one for implementing caps, the other one for using them?

nothingmuch commented 2 years ago

Feel free to go ahead with partial.

Cool, I'll open a PR hopefully in the next few days

Does Foo implement IFooReader? Does Reader implement IFooReader?

Yes to both

As said, I don't see any easy way to unite Foo and Foo.READER in a common interface.

The interface should only consist of the parts that already overlap. For the following definition of Foo:


struct Foo {
   foo @0 :Int32;
}

IFooReader would consist only of the getters on the properties:

public interface IFooReader { int Foo { get; } }

and nothing about construction.

IFooWriter or IFoo would also require a setter for that int property:

public interface IFoo: IFooReader { int Foo { set; } }

And the domain classes would have the same implementation apart from these two additional interfaces:

public struct Foo : IFoo
{
    public struct READER : IFooReader { ... }
    public struct WRITER: IFoo { ... }
    ...
}

Given a signature like Task Frobnicate(Foo.Reader foo), how should this look like on the caller side? Would we expect callers to provide a Foo.Reader

To me specifying Foo.Reader is a clear indication that the user is deliberately targeting a narrower interface attempting to control serialization of data. I expect the common cases would be expecting a Foo (as right now) or an IFooReader (Foo.IReader?) which is a broader interface. An explicit Foo.Reader likely has a specific implementation goal that is probably more considered so avoiding automagical behavior in that case seems like the right choice to me, and in that case the implementor should only be usable if an actual Foo.Reader, and indeed it's it's not a behavioral subtype of Foo. If the user really intended to implement that and then really intended to call it from code that already has a different notion of Foo, that Foo is already an IFooReader and therefore its data could be copied into a new writer in order to construct a real segment to back a real Foo.Reader if that's really the intended behavior.