capnproto-rust icon indicating copy to clipboard operation
capnproto-rust copied to clipboard

Help improving performance

Open elferherrera opened this issue 3 years ago • 6 comments

Hi developers,

First, let me thank you for this crate. I have been using it for the past weeks and the interface is very well thought. It really helped me to flesh out some ideas we want to implement in nushell really quickly.

My issue is related to performance. We want to use a fast serializer to be the backbone for several components we want to add to nushell, one of those being a plugin module. The idea behind this module is to be able to create external commands that can be registered into the shell engine and be used as normal commands. You can have a look at my first attempt here.

The module works and a test plugin can be found here. Unfortunately the performance is not what I was expecting. The previous serializer engine, which uses bjson to share the messages, is slightly faster than this new one (engine-q is the new engine we are working on)

image

I'm wondering if you can give us a hand. Do you mind checking this folder to see if I didnt screw up the implementation? Since this is the first time I use capnp I don't know if I'm using all the right tricks to use it as efficiently as possible.

Thanks in advance

elferherrera avatar Nov 05 '21 06:11 elferherrera

Some things that I noticed on a quick look:

  1. You may be doing more allocations than you need to. For message construction, try experimenting with different values for first_segment_words: https://github.com/capnproto/capnproto-rust/blob/0f7abd17ba653ea2130185ca20ebb953dc729c5e/capnp/src/message.rs#L439. You might also try experiment with reusing the first segment via ScratchSpaceHeapAllocator: https://github.com/capnproto/capnproto-rust/blob/0f7abd17ba653ea2130185ca20ebb953dc729c5e/capnp/src/message.rs#L516-L526
  2. unpacked encoding will be faster than packed encoding (but will of course be less compact)
  3. struct Option(T) is inefficient in capnp, requiring an extra 8 bytes. You don't actually need to wrap fields in Optional to make them optional. See https://capnproto.org/faq.html#how-do-i-make-a-field-optional.

dwrensha avatar Nov 06 '21 00:11 dwrensha

Hi,

Thanks a lot for the pointers. Im looking at them right now.

Regarding the Option suggestion, how do you normally manage a non common optional value. Let's say I have a struct that is optional in another struct. How could that be a bogus value when it has to have a specific shape? do you have an example for this?

elferherrera avatar Nov 06 '21 11:11 elferherrera

For example, you currently have

struct Signature {
    name @0 :Text;
    usage @1 :Text;
    extraUsage @2 :Text;
    requiredPositional @3 :List(Argument);
    optionalPositional @4 :List(Argument);
    rest @5 :Option(Argument);
    named @6 :List(Flag);
    isFilter @7 :Bool;
}

That could instead be

struct Signature {
    name @0 :Text;
    usage @1 :Text;
    extraUsage @2 :Text;
    requiredPositional @3 :List(Argument);
    optionalPositional @4 :List(Argument);
    rest @5 :Argument;
    named @6 :List(Flag);
    isFilter @7 :Bool;
}

as long has you call signature_reader.has_rest() to check for nullness.

dwrensha avatar Nov 06 '21 14:11 dwrensha

Thanks. Gotcha.

I have already implemented the change of serializer and there was a slight improvement. Im going to try now the optional change.

I couldn't understand you first two suggestions. How does the HeapAllocator work with the builder?

elferherrera avatar Nov 06 '21 17:11 elferherrera

You can pass a HeapAllocator into message::Builder::new():

https://github.com/capnproto/capnproto-rust/blob/9cf00528a6a92ba57c835fd467f5217d84e5707b/capnp/src/message.rs#L372

The default first segment size is 1024 words. If your messages are significantly smaller than that it could help to make the first segment smaller. https://github.com/capnproto/capnproto-rust/blob/9cf00528a6a92ba57c835fd467f5217d84e5707b/capnp/src/message.rs#L651-L655 https://github.com/capnproto/capnproto-rust/blob/9cf00528a6a92ba57c835fd467f5217d84e5707b/capnp/src/message.rs#L572-L578

dwrensha avatar Nov 07 '21 00:11 dwrensha

@elferherrera did you ever get this ironed out? Managing to be slower than bjson is pretty surprising, though I guess if using packed encoding it depends on how many of your fields you typically populate. A flame graph would probably go a long way.

haydenflinner avatar Feb 08 '23 14:02 haydenflinner