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

x/evm/keeper: use the fastest slice making idiom for Keeper.EthereumTx.Logs #827

Merged
merged 1 commit into from
Dec 14, 2021

Conversation

odeke-em
Copy link
Contributor

Uses the fastest slice making idiom of creating the well known
size of a slice using

make([]sdk.Attribute, len(response.Logs))
for i, log := range response.Logs {
    txLogAttrs[i] = ...
}

instead of

make([]sdk.Attribute, 0)
for _, log := range response.Logs {
    txLogAttrs = append(txLogAttrs, ...)
}

which had a few problems:

  1. Using 0 for size then appending is quite slow yet we know the exact size
  2. Using append instead of indexing is slower

If we examine the advisory at https://bencher.orijtech.com/perfclinic/sliceupdate/
and the verdict at https://bencher.orijtech.com/perfclinic/sliceupdate/#verdict
this new scheme shows a massive improvement in that call site.

Fixes #825

Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

LGTM, can you add a changelog entry?

@codecov
Copy link

codecov bot commented Dec 13, 2021

Codecov Report

Merging #827 (6b265ab) into main (4ee1d86) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 6b265ab differs from pull request most recent head 46a47df. Consider uploading reports for the commit 46a47df to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main     #827   +/-   ##
=======================================
  Coverage   57.31%   57.31%           
=======================================
  Files          72       72           
  Lines        5984     5984           
=======================================
  Hits         3430     3430           
  Misses       2357     2357           
  Partials      197      197           
Impacted Files Coverage Δ
x/evm/keeper/msg_server.go 73.58% <100.00%> (ø)

…x.Logs

Uses the fastest slice making idiom of creating the well known
size of a slice using

    make([]sdk.Attribute, len(response.Logs))
    for i, log := range response.Logs {
        txLogAttrs[i] = ...
    }

instead of

    make([]sdk.Attribute, 0)
    for _, log := range response.Logs {
        txLogAttrs = append(txLogAttrs, ...)
    }

which had a few problems:
1. Using 0 for size then appending is quite slow yet we know the exact size
2. Using append instead of indexing is slower

If we examine the advisory at https://bencher.orijtech.com/perfclinic/sliceupdate/
and the verdict at https://bencher.orijtech.com/perfclinic/sliceupdate/#verdict
this new scheme shows a massive improvement in that call site.

Fixes #825
@odeke-em odeke-em force-pushed the evm-keeper-use-faster-slice-maker branch from 6b265ab to 46a47df Compare December 14, 2021 00:01
@odeke-em
Copy link
Contributor Author

LGTM, can you add a changelog entry?

Done, please take a look and thank you @fedekunze!

@odeke-em odeke-em enabled auto-merge (squash) December 14, 2021 00:02
@odeke-em odeke-em merged commit a2f246c into main Dec 14, 2021
@odeke-em odeke-em deleted the evm-keeper-use-faster-slice-maker branch December 14, 2021 00:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x/evm: (*Keeper).EthereumTx should use make(T, len) instead of make(T, 0)
2 participants