Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Optimize DIV for some common divisors #2327

Merged
merged 1 commit into from
Sep 27, 2016
Merged

Optimize DIV for some common divisors #2327

merged 1 commit into from
Sep 27, 2016

Conversation

arkpar
Copy link
Collaborator

@arkpar arkpar commented Sep 26, 2016

No description provided.

@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. B0-patch labels Sep 26, 2016
@rphmeier
Copy link
Contributor

how were these divisors selected? benchmark results?

TWO_POW_96 => a >> 96,
TWO_POW_224 => a >> 224,
TWO_POW_248 => a >> 248,
_ => a.overflowing_div(b).0,
Copy link
Contributor

Choose a reason for hiding this comment

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

couldn't these be put into the implementation of div itself for U256?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's better here - it's for evm's specific use cases. Uint is supposed to be general purpose library I don't think it suits well there. Probably would be better to use a different division algorithm there in additon.

@gavofyork gavofyork added M4-core ⛓ Core client code / Rust. A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Sep 26, 2016
@gavofyork gavofyork modified the milestone: 1.3.2 Sep 26, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 85.206% when pulling 965ff93 on div-optimize into 92451ef on master.

@arkpar
Copy link
Collaborator Author

arkpar commented Sep 27, 2016

Selected by building a histogram for a few thousand recent blocks. This improves import time for some of the heavy blocks by a factor of two

@gavofyork gavofyork merged commit bc4cbaa into master Sep 27, 2016
@gavofyork gavofyork deleted the div-optimize branch September 27, 2016 09:27
@arkpar arkpar changed the title Optimize DIV for some common divizors Optimize DIV for some common divisors Sep 27, 2016
@gavofyork gavofyork removed this from the 1.3.2 milestone Sep 27, 2016
jacogr added a commit that referenced this pull request Sep 29, 2016
* master:
  Fixing Delegate Call in JIT (#2378)
  Prioritizing re-imported transactions (#2372)
  Revert #2172, pretty much. (#2387)
  correct sync memory usage calculation (#2385)
  Update gitlab-ci
  Fix the traceAddress field in transaction traces. (#2373)
  Removing extras data from retracted blocks. (#2375)
  fixed #2263, geth keys with ciphertext shorter than 32 bytes (#2318)
  Expanse compatibility (#2369)
  Specify column cache sizes explicitly; default fallback of 2MB (#2358)
  Canonical state cache (master) (#2311)
  make block queue into a more generic verification queue and fix block heap size calculation (#2095)
  Hash Content RPC method (#2355)
  Reorder transaction_by_hash to favour canon search (#2332)
  DIV optimization (#2327)
  Error when deserializing invalid hex (#2339)
  Changed http:// to https:// on some links (#2349)
  add a test
  fix migration system, better errors

# Conflicts:
#	.gitlab-ci.yml
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants