Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

x/evm/keeper: save 24B with Go in-condition variable idiom #828

Merged
merged 5 commits into from
Dec 13, 2021

Conversation

odeke-em
Copy link
Contributor

The prior code doubly invoked (*ethereum/go-ethereum/core/types.Transaction).To()
which is quite expensive, and firstly copies 20 bytes each time, then
that gets rounded up to the proper size class/pointer alignment so on
64-bit machines 20B -> 24B

Isolating a benchmark for this code per issue #826 shows this saves
quite a bit of bytes and some nano seconds which all count up towards
the transactions per seconds being processed:

$ benchstat before.txt after.txt
name        old time/op    new time/op    delta
CopyAddr-8    38.4ns ± 3%    19.3ns ± 3%  -49.66%  (p=0.000 n=10+10)

name        old alloc/op   new alloc/op   delta
CopyAddr-8     48.0B ± 0%     24.0B ± 0%  -50.00%  (p=0.000 n=10+10)

name        old allocs/op  new allocs/op  delta
CopyAddr-8      2.00 ± 0%      1.00 ± 0%  -50.00%  (p=0.000 n=10+10)

Fixes #826

@codecov
Copy link

codecov bot commented Dec 12, 2021

Codecov Report

Merging #828 (e98d76f) into main (b6b4ae8) will not change coverage.
The diff coverage is 100.00%.

❗ Current head e98d76f differs from pull request most recent head 348ad0a. Consider uploading reports for the commit 348ad0a to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main     #828   +/-   ##
=======================================
  Coverage   57.30%   57.30%           
=======================================
  Files          73       73           
  Lines        6029     6029           
=======================================
  Hits         3455     3455           
  Misses       2374     2374           
  Partials      200      200           
Impacted Files Coverage Δ
x/evm/keeper/msg_server.go 73.58% <100.00%> (ø)

@odeke-em odeke-em force-pushed the evm-keeper-singly-create-To-save-24B branch from 392cc3b to ae26b42 Compare December 13, 2021 02:02
@github-actions github-actions bot added the Type: CI continuous integration label Dec 13, 2021
@odeke-em odeke-em force-pushed the evm-keeper-singly-create-To-save-24B branch 2 times, most recently from 29ed0ad to e98d76f Compare December 13, 2021 19:40
odeke-em and others added 4 commits December 13, 2021 14:10
The prior code doubly invoked (*ethereum/go-ethereum/core/types.Transaction).To()
which is quite expensive, and firstly copies 20 bytes each time, then
that gets rounded up to the proper size class/pointer alignment so on
64-bit machines 20B -> 24B.

Isolating a benchmark for this code per issue #826 shows this saves
quite a bit of bytes and some nano seconds which all count up towards
the transactions per seconds being processed:

```shell
$ benchstat before.txt after.txt
name        old time/op    new time/op    delta
CopyAddr-8    38.4ns ± 3%    19.3ns ± 3%  -49.66%  (p=0.000 n=10+10)

name        old alloc/op   new alloc/op   delta
CopyAddr-8     48.0B ± 0%     24.0B ± 0%  -50.00%  (p=0.000 n=10+10)

name        old allocs/op  new allocs/op  delta
CopyAddr-8      2.00 ± 0%      1.00 ± 0%  -50.00%  (p=0.000 n=10+10)
```

Fixes #826
@odeke-em odeke-em force-pushed the evm-keeper-singly-create-To-save-24B branch from e98d76f to e516c3c Compare December 13, 2021 22:10
@fedekunze fedekunze enabled auto-merge (squash) December 13, 2021 23:33
@fedekunze fedekunze enabled auto-merge (squash) December 13, 2021 23:48
@fedekunze fedekunze enabled auto-merge (squash) December 13, 2021 23:50
@fedekunze fedekunze merged commit 8333765 into main Dec 13, 2021
@fedekunze fedekunze deleted the evm-keeper-singly-create-To-save-24B branch December 13, 2021 23:51
odeke-em added a commit that referenced this pull request Dec 14, 2021
Following suit with PR #828, this change cuts down the expenses
from using .To doubly; yet using the Go in-condition variable idiom.

Updates #826
odeke-em added a commit that referenced this pull request Dec 14, 2021
Following suit with PR #828, this change cuts down the expenses
from using .To doubly; yet using the Go in-condition variable idiom.

Updates #826
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: CI continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x/evm/keeper: invoke tx.To() once and reuse that value instead of incurring a 24B allocation for a [20]byte
3 participants