Skip to content

IPC protocol should be buffer based, not JSON based#56820

Merged
joaomoreno merged 5 commits intomasterfrom
joao/ipc-buffers
Aug 21, 2018
Merged

IPC protocol should be buffer based, not JSON based#56820
joaomoreno merged 5 commits intomasterfrom
joao/ipc-buffers

Conversation

@joaomoreno
Copy link
Member

@joaomoreno joaomoreno commented Aug 20, 2018

  • Make IPC Buffer based, instead of any based
  • Provide better buffer serialization for buffer data arguments

@joaomoreno joaomoreno added feature-request Request for new features or functionality ipc labels Aug 20, 2018
@joaomoreno joaomoreno added this to the August 2018 milestone Aug 20, 2018
@joaomoreno joaomoreno self-assigned this Aug 20, 2018
@joaomoreno joaomoreno requested a review from sandy081 August 20, 2018 13:50
@joaomoreno
Copy link
Member Author

Pushed the first step. This allows us full control at the protocol layer as to how to serialize/deserialize things; namely, if that thing is a buffer, everything's easier.

One issue emerged. Because the abstraction is one layer below, we possibly lost performance on two areas:

  • IPC over child processes using fork (ipc.cp.ts)
  • IPC over Electron

Both these implementations do not support Buffers currently. Here's my proposal for each:

  • Remove protocol work from ipc.cp.ts and make it dependent on ipc.net.ts, since that implementation works using named pipes and sockets. This will let us be independent from node's fork and serialization, which is a good thing anyway.
  • Wait for Electron 3 which brings Buffer support over IPC: Fix support for typed arrays in remote/rpc-server electron/electron#13055. This is fine since we send very small and few messages over Electron IPC. Another alternative here is to also ditch Electron IPC and just use ipc.net.ts as well.

@joaomoreno
Copy link
Member Author

joaomoreno commented Aug 20, 2018

Second step pushed. Buffers can now be used in the following places:

  • Promise request arg
  • Promise resolution result
  • Event request arg
  • Event data

@microsoft microsoft deleted a comment from Ngocbich95 Aug 20, 2018
@joaomoreno joaomoreno merged commit d259a97 into master Aug 21, 2018
@joaomoreno joaomoreno deleted the joao/ipc-buffers branch August 21, 2018 08:00
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature-request Request for new features or functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants