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

rlp: minor optimizations for slice/array encoding #23467

Merged
merged 4 commits into from
Aug 25, 2021
Merged

Conversation

fjl
Copy link
Contributor

@fjl fjl commented Aug 25, 2021

As per benchmark results below, these changes speed up encoding/decoding of consensus objects a bit.

name                             old time/op    new time/op    delta
EncodeRLP/legacy-header-8           384ns ± 1%     331ns ± 3%  -13.83%  (p=0.000 n=7+8)
EncodeRLP/london-header-8           411ns ± 1%     359ns ± 2%  -12.53%  (p=0.000 n=8+8)
EncodeRLP/receipt-for-storage-8     251ns ± 0%     239ns ± 0%   -4.97%  (p=0.000 n=8+8)
EncodeRLP/receipt-full-8            319ns ± 0%     300ns ± 0%   -5.89%  (p=0.000 n=8+7)
EncodeRLP/legacy-transaction-8      389ns ± 1%     387ns ± 1%     ~     (p=0.099 n=8+8)
EncodeRLP/access-transaction-8      607ns ± 0%     581ns ± 0%   -4.26%  (p=0.000 n=8+8)
EncodeRLP/1559-transaction-8        627ns ± 0%     606ns ± 1%   -3.44%  (p=0.000 n=8+8)
DecodeRLP/legacy-header-8           831ns ± 1%     813ns ± 1%   -2.20%  (p=0.000 n=8+8)
DecodeRLP/london-header-8           824ns ± 0%     804ns ± 1%   -2.44%  (p=0.000 n=8+7)

I created these changes before starting work on the code generator. Mostly submitting because it took non-trivial effort to find these optimizations, so I feel like the changes are valuable even if we end up not using the reflect-based encoder/decoder later.

This makes it possible to inline the function, and the length
is known at encoder construction time.
It's actually cheaper to use Elem first, because it performs less checks
on the value. If the pointer was nil, the result of Elem is 'invalid'.
For empty slices/arrays, we can avoid storing a list header entry in the
encoder buffer. Also avoid doing the tail check at encoding time because
it is already known at encoder construction time.
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fjl fjl merged commit 32c576b into ethereum:master Aug 25, 2021
@fjl fjl added this to the 1.10.9 milestone Aug 25, 2021
sidhujag pushed a commit to sidhujag/go-ethereum that referenced this pull request Aug 25, 2021
As per benchmark results below, these changes speed up encoding/decoding of
consensus objects a bit.

    name                             old time/op    new time/op    delta
    EncodeRLP/legacy-header-8           384ns ± 1%     331ns ± 3%  -13.83%  (p=0.000 n=7+8)
    EncodeRLP/london-header-8           411ns ± 1%     359ns ± 2%  -12.53%  (p=0.000 n=8+8)
    EncodeRLP/receipt-for-storage-8     251ns ± 0%     239ns ± 0%   -4.97%  (p=0.000 n=8+8)
    EncodeRLP/receipt-full-8            319ns ± 0%     300ns ± 0%   -5.89%  (p=0.000 n=8+7)
    EncodeRLP/legacy-transaction-8      389ns ± 1%     387ns ± 1%     ~     (p=0.099 n=8+8)
    EncodeRLP/access-transaction-8      607ns ± 0%     581ns ± 0%   -4.26%  (p=0.000 n=8+8)
    EncodeRLP/1559-transaction-8        627ns ± 0%     606ns ± 1%   -3.44%  (p=0.000 n=8+8)
    DecodeRLP/legacy-header-8           831ns ± 1%     813ns ± 1%   -2.20%  (p=0.000 n=8+8)
    DecodeRLP/london-header-8           824ns ± 0%     804ns ± 1%   -2.44%  (p=0.000 n=8+7)

* rlp: pass length to byteArrayBytes

This makes it possible to inline byteArrayBytes. For arrays, the length is known
at encoder construction time, so the call to v.Len() can be avoided.

* rlp: avoid IsNil for pointer encoding

It's actually cheaper to use Elem first, because it performs less checks
on the value. If the pointer was nil, the result of Elem is 'invalid'.

* rlp: minor optimizations for slice/array encoding

For empty slices/arrays, we can avoid storing a list header entry in the
encoder buffer. Also avoid doing the tail check at encoding time because
it is already known at encoder construction time.
yongjun925 pushed a commit to DODOEX/go-ethereum that referenced this pull request Dec 3, 2022
As per benchmark results below, these changes speed up encoding/decoding of
consensus objects a bit.

    name                             old time/op    new time/op    delta
    EncodeRLP/legacy-header-8           384ns ± 1%     331ns ± 3%  -13.83%  (p=0.000 n=7+8)
    EncodeRLP/london-header-8           411ns ± 1%     359ns ± 2%  -12.53%  (p=0.000 n=8+8)
    EncodeRLP/receipt-for-storage-8     251ns ± 0%     239ns ± 0%   -4.97%  (p=0.000 n=8+8)
    EncodeRLP/receipt-full-8            319ns ± 0%     300ns ± 0%   -5.89%  (p=0.000 n=8+7)
    EncodeRLP/legacy-transaction-8      389ns ± 1%     387ns ± 1%     ~     (p=0.099 n=8+8)
    EncodeRLP/access-transaction-8      607ns ± 0%     581ns ± 0%   -4.26%  (p=0.000 n=8+8)
    EncodeRLP/1559-transaction-8        627ns ± 0%     606ns ± 1%   -3.44%  (p=0.000 n=8+8)
    DecodeRLP/legacy-header-8           831ns ± 1%     813ns ± 1%   -2.20%  (p=0.000 n=8+8)
    DecodeRLP/london-header-8           824ns ± 0%     804ns ± 1%   -2.44%  (p=0.000 n=8+7)

* rlp: pass length to byteArrayBytes

This makes it possible to inline byteArrayBytes. For arrays, the length is known
at encoder construction time, so the call to v.Len() can be avoided.

* rlp: avoid IsNil for pointer encoding

It's actually cheaper to use Elem first, because it performs less checks
on the value. If the pointer was nil, the result of Elem is 'invalid'.

* rlp: minor optimizations for slice/array encoding

For empty slices/arrays, we can avoid storing a list header entry in the
encoder buffer. Also avoid doing the tail check at encoding time because
it is already known at encoder construction time.
nibty pushed a commit to FairCrypto/go-ethereum that referenced this pull request Apr 10, 2024
As per benchmark results below, these changes speed up encoding/decoding of
consensus objects a bit.

    name                             old time/op    new time/op    delta
    EncodeRLP/legacy-header-8           384ns ± 1%     331ns ± 3%  -13.83%  (p=0.000 n=7+8)
    EncodeRLP/london-header-8           411ns ± 1%     359ns ± 2%  -12.53%  (p=0.000 n=8+8)
    EncodeRLP/receipt-for-storage-8     251ns ± 0%     239ns ± 0%   -4.97%  (p=0.000 n=8+8)
    EncodeRLP/receipt-full-8            319ns ± 0%     300ns ± 0%   -5.89%  (p=0.000 n=8+7)
    EncodeRLP/legacy-transaction-8      389ns ± 1%     387ns ± 1%     ~     (p=0.099 n=8+8)
    EncodeRLP/access-transaction-8      607ns ± 0%     581ns ± 0%   -4.26%  (p=0.000 n=8+8)
    EncodeRLP/1559-transaction-8        627ns ± 0%     606ns ± 1%   -3.44%  (p=0.000 n=8+8)
    DecodeRLP/legacy-header-8           831ns ± 1%     813ns ± 1%   -2.20%  (p=0.000 n=8+8)
    DecodeRLP/london-header-8           824ns ± 0%     804ns ± 1%   -2.44%  (p=0.000 n=8+7)

* rlp: pass length to byteArrayBytes

This makes it possible to inline byteArrayBytes. For arrays, the length is known
at encoder construction time, so the call to v.Len() can be avoided.

* rlp: avoid IsNil for pointer encoding

It's actually cheaper to use Elem first, because it performs less checks
on the value. If the pointer was nil, the result of Elem is 'invalid'.

* rlp: minor optimizations for slice/array encoding

For empty slices/arrays, we can avoid storing a list header entry in the
encoder buffer. Also avoid doing the tail check at encoding time because
it is already known at encoder construction time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants