Add protons back

[twoeths]

trafficstars

Motivation

In #310 we have to switch back to protobufjs because protons is 20x slower than protobufjs, we should add it back once the performance of protons is comparable or better than protobufjs

Description Use the following benchmark (need to add "@dapplion/benchmark" as devDependencies):

import { itBench, setBenchOpts } from '@dapplion/benchmark'
import { RPC } from '../../../src/message/rpc.js'

/**
 *  rpc
    ✔ decode Attestation message                                           6807815 ops/s    146.8900 ns/op        -    1977187 runs   30.1 s
 */
describe.only('rpc', function () {
  this.timeout(0)
  setBenchOpts({
    maxMs: 60 * 1000,
    minMs: 30 * 1000,
    minRuns: 200
  })

  const rpc: RPC = {
    subscriptions: [],
    messages: [
      {
        topic: 'topic1',
        // typical Attestation
        data: Buffer.from(
          'e40000000a000000000000000a00000000000000a45c8daa336e17a150300afd4c717313c84f291754c51a378f20958083c5fa070a00000000000000a45c8daa336e17a150300afd4c717313c84f291754c51a378f20958083c5fa070a00000000000000a45c8daa336e17a150300afd4c717313c84f291754c51a378f20958083c5fa0795d2ef8ae4e2b4d1e5b3d5ce47b518e3db2c8c4d082e4498805ac2a686c69f248761b78437db2927470c1e77ede9c18606110faacbcbe4f13052bde7f7eff6aab09edf7bc4929fda2230f943aba2c47b6f940d350cb20c76fad4a8d40e2f3f1f01',
          'hex'
        ),
        signature: Uint8Array.from(Array.from({ length: 96 }, () => 100))
      }
    ],
    control: undefined
  }

  const bytes = RPC.encode(rpc)

  console.log('@@@ encoded to', Buffer.from(bytes.slice()).toString('hex'), 'length', bytes.length)

  itBench({
    id: 'decode Attestation message',
    fn: () => {
      RPC.decode(bytes)
    },
    runsFactor: 100
  })
})

need to also add similar benchmark for encode

[achingbrain]

I have incorporated the above benchmark into protons and running it with the latest version (5.1.0) yields good results:

% node packages/protons-benchmark/dist/src/rpc.js
protons x 3,174,965 ops/sec ±0.40% (87 runs sampled)
protobufjs x 2,976,375 ops/sec ±1.72% (89 runs sampled)
Fastest is protons

[twoeths]

thanks @achingbrain , protons 5.1.0 is really faster than older versions however it's not very stable, see the statistics here https://github.com/ChainSafe/js-libp2p-gossipsub/pull/327

[achingbrain]

That's interesting, [email protected] uses the Reader and Writer classes from [email protected], are you sure something else isn't affecting the benchmark run?

Also, how are you generating those stats, I can't see anything new in the benchmark folder?

[twoeths]

Also, how are you generating those stats, I can't see anything new in the benchmark folder?

I just run the test in my local environment

That's interesting, [email protected] uses the Reader and Writer classes from [email protected], are you sure something else isn't affecting the benchmark run?

I cannot think of anything that affect the benchmark but we can only be sure if we track the benchmark as part of CI steps. But I didn't see that same issue with protobufjs

@achingbrain If you have a chance please run it multiple times in your environment too (using #327) to see if the issue happens or not

[twoeths]

Got memory leak issue with protons 5.1.0 that causes lodestar to be restarted multiple times, this is from a test mainnet node subscribing to all subnets:

[achingbrain]

Are you sure this is protons related? protons 7.x shipped six months ago, has 20-30k downloads a week, is used all over the libp2p/ipfs/helia stack and in a bunch of other projects and this hasn't been reported.

[twoeths]

Are you sure this is protons related? protons 7.x shipped six months ago, has 20-30k downloads a week, is used all over the libp2p/ipfs/helia stack and in a bunch of other projects and this hasn't been reported.

@achingbrain I haven't used protons 7.x as it only supports proto3. This happens with protons 5.1.0 and protons-runtime 3.1.0

[achingbrain]

src/message/rpc.proto is already proto3 syntax so I'm not sure anything is blocked here?

[achingbrain]

Actually looking at this more, this is proto2 with the syntax field set to proto3.

1. There is an error on this line - proto3 does not have a required type.
   • By default fields are not sent on the wire if they are set to the default value (0 for most number types, etc) but in proto2 it meant that the value always has to be sent, otherwise the message is invalid.
   • You can simulate this in proto3 by marking the field optional and ensuring the value is always set.
2. In proto3 the default type is singular, this means if the field is set to the default value it is not sent on the wire, at the other end if the field is not on the wire it's set to the default value - e.g. there's no way to tell if the value was sent or not
   • This is how optional behaved in proto2

In proto3 the optional type means if the field is not set, no value is sent on the wire. If the field was explicitly set or parsed from the wire it will be returned, and you can check to see if the value was explicitly set

• In proto3 this lets us detect if a field was not set
• There is no equivalent in proto2
• In proto2 a default type is always returned, even for fields marked "optional" in the proto2 .proto file

TLDR for converting from proto2 to proto3 is:

1. Remove optional keyword (proto3 "singular" behaves like proto2 "optional")
2. Add optional keyword to fields that were required in proto2 and ensure they are set at the application level

[twoeths]

the memory leak was lodestar issue of not using correct NodeJS version, see https://github.com/ChainSafe/lodestar/issues/5851

@achingbrain thanks for the migration guide. I updated to proto3 using protons 7.0.5, and it requires protons-runtime version 5, and it's 2x slower compared to protons-runtime 3.x as in https://github.com/ChainSafe/js-libp2p-gossipsub/pull/327#issuecomment-1658059241 (you can give it a try in tuyen/protons_7 branch)

do you have any ideas? should I go with proto2 + protons 5 + protons-runtime 3 as in the PR or wait for an improvement in protons-runtime?

[twoeths]

created https://github.com/ipfs/protons/issues/109 to track the performance issue of protons-runtime v5

[twoeths]

#327 is ready to test once lodestar migrated to latest libp2p https://github.com/ChainSafe/lodestar/pull/5869