Skip to content
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

Performance: Avoid converting responses to serde_json::Value #639

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Commits on Oct 24, 2021

  1. Performance: Avoid converting responses to serde_json::Value

    Previously, all responses were converted to serde_json::Value before
    being serialized to string. Since jsonrpc does not inspect the result
    object in any way, this step could be skipped.
    
    Now, result objects are serialized to string much earlier and the Value
    step is skipped.
    
    This patch has large performance benefits for huge responses: In tests
    with 4.5 GB responses, the jsonrpc serialization overhead after
    returning from an rpc function was reduced by around 35%. Which means
    several seconds of speed-up in response times.
    
    To accomplish this while mostly retaining API compatibility, the traits
    RpcMethod, RpcMethodSync, RpcMethodSimple are now generic in their
    return type and are wrapped when added to an io handler.
    
    There's now a distinction between the parsed representation of jsonrpc
    responses (Output/Response) and result of rpc functions calls
    (WrapOutput/WrapResponse) to allow the latter to carry the
    pre-serialized strings.
    
    Review notes:
    - I'm not happy with the WrapOutput / WrapResponse names, glad for
      suggestions.
    - Similarly, rpc_wrap must be renamed and moved.
    - The http handler health request is now awkward, and must extract the
      result/error from the already-serialized response. Probably worth it.
    - The largest API breakage I could see is in the middleware, where the
      futures now return WrapOutput/WrapResult instead of Output/Result.
    - One could make WrapOutput just be String, but having a separate type
      is likely easier to read.
    
    See paritytech#212
    ckamm committed Oct 24, 2021
    Configuration menu
    Copy the full SHA
    1839c50 View commit details
    Browse the repository at this point in the history

Commits on Nov 1, 2021

  1. review: renames

    - WrapOutput -> SerializedOutput
    - WrapResponse -> SerializedResponse
    - RpcMethodWrapped -> RpcMethodWithSerializedOutput
    ckamm committed Nov 1, 2021
    Configuration menu
    Copy the full SHA
    e2d86df View commit details
    Browse the repository at this point in the history

Commits on Nov 2, 2021

  1. Configuration menu
    Copy the full SHA
    aeeac66 View commit details
    Browse the repository at this point in the history

Commits on Nov 4, 2021

  1. Update core/src/calls.rs

    Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>
    ckamm and tomusdrw committed Nov 4, 2021
    Configuration menu
    Copy the full SHA
    9770c0b View commit details
    Browse the repository at this point in the history
  2. Update core/src/calls.rs

    Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>
    ckamm and tomusdrw committed Nov 4, 2021
    Configuration menu
    Copy the full SHA
    c3f9a2b View commit details
    Browse the repository at this point in the history