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

util: optimize hexToBytes #3203

Merged
merged 3 commits into from
Dec 19, 2023
Merged

util: optimize hexToBytes #3203

merged 3 commits into from
Dec 19, 2023

Conversation

jochem-brouwer
Copy link
Member

This PR optimizes hexToBytes.

NOTE: I also changed unprefixedHexToBytes to use this, because we call unprefixedHexToBytes in toBytes in case we convert a BigInt to Bytes!

To test:

import { hexToBytes as hexToBytesCrypto } from 'ethereum-cryptography/utils'
import { hexToBytes, padToEven } from '../src'

const allHexChars = 'aabbccddeeffAABBCCDDEEFF00112233445566778899'
const storageSlotMax = '0x' + ('ff').repeat(32)
const cases = ['0x', '0xaA', storageSlotMax, '0x' + allHexChars, '0x' + allHexChars.repeat(32)]
const iter = 1_000_000

const oldHexToBytes = (hex: string): Uint8Array => {
  if (typeof hex !== 'string') {
    throw new Error(`hex argument type ${typeof hex} must be of type string`)
  }

  if (!/^0x[0-9a-fA-F]*$/.test(hex)) {
    throw new Error(`Input must be a 0x-prefixed hexadecimal string, got ${hex}`)
  }

  hex = hex.slice(2)

  if (hex.length % 2 !== 0) {
    hex = padToEven(hex)
  }

  const byteLen = hex.length
  const bytes = new Uint8Array(byteLen / 2)
  for (let i = 0; i < byteLen; i += 2) {
    const byte = parseInt(hex.slice(i, i + 2), 16)
    bytes[i / 2] = byte
  }
  return bytes
}

function benchOld(hex: string) {
  oldHexToBytes(hex)
}

function benchNew(hex: string) {
  hexToBytes(hex)
}

function benchEthCrypo(hex: string) {
    hexToBytesCrypto(hex)
}

function loop(func: Function, name?: string, warmup: boolean = true) {
  let testC = 0
  for (const input of cases) {
    if (!warmup) console.time(name + ' - ' + testC)
    else {
      console.log(name + ' - ' + testC)
    }
    for (let i = 0; i < iter; i++) {
      func(input)
    }

    if (!warmup) console.timeEnd(name + ' - ' + testC)

    testC++
  }
}

loop(benchOld, 'old - warmup')
loop(benchEthCrypo, 'oldCrypto - warmup')
loop(benchNew, 'new - warmup')

loop(benchOld, 'old', false)
loop(benchEthCrypo, 'oldCrypto', false)
loop(benchNew, 'new', false)

Sample output;

old - 0: 71.902ms
old - 1: 128.238ms
old - 2: 1.675s
old - 3: 1.160s
old - 4: 35.893s
oldCrypto - 0: 63.829ms
oldCrypto - 1: 148.783ms
oldCrypto - 2: 2.083s
oldCrypto - 3: 1.534s
oldCrypto - 4: 42.828s
new - 0: 60.571ms
new - 1: 113.212ms
new - 2: 907.089ms
new - 3: 809.084ms
new - 4: 23.068s

Note: the test marked with "2" is a test on the largest item in the EVM (so 2^256 - 1). In bigIntToBytes we eventually call hexToBytes with a potential speedup here of ~45% !?

Please review thorougly.

Copy link

codecov bot commented Dec 16, 2023

Codecov Report

Merging #3203 (7364e32) into master (4f908ca) will increase coverage by 0.11%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 87.52% <ø> (ø)
blockchain 91.60% <ø> (ø)
client ?
common 98.25% <ø> (ø)
devp2p ?
ethash ?
evm 77.42% <ø> (ø)
statemanager 86.29% <ø> (ø)
trie ?
tx ?
util 89.14% <100.00%> (+0.08%) ⬆️
vm 80.83% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

LGTM! Got similar results
image

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

LGTM from my side as well, great and deep reaching work! Will merge.

@holgerd77 holgerd77 merged commit c5054eb into master Dec 19, 2023
46 checks passed
@holgerd77 holgerd77 deleted the optimize-hextobytes branch December 19, 2023 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants