Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Correct cpu_usage calculation when more than one signature #7809

Merged
merged 3 commits into from
Aug 28, 2019

Conversation

heifner
Copy link
Contributor

@heifner heifner commented Aug 27, 2019

Change Description

  • CPU usage of signature recovery was not being calculated correctly for multiple signatures. Correct calculation so that multiple signature recovery time is not overly calculated.

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

Copy link
Contributor

@b1bart b1bart left a comment

Choose a reason for hiding this comment

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

No need to block this PR as cpu billing is subjective however, this is a "change of policy" of sorts and we should consider documenting it somewhere or maintaining the old "policy"

@@ -712,10 +712,6 @@ BOOST_AUTO_TEST_CASE(transaction_test) { try {
BOOST_CHECK(cpu_time1 > fc::microseconds(0));
BOOST_CHECK(cpu_time2 > fc::microseconds(0));

// verify that hitting cache still indicates same billable time
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this check had to go because the digest_time was not exactly equivalent and we were already verifying that cpu_time2 was >0?

Is there any value in maintaining the invariant that cache hits/misses bill exactly the same amount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see any value in maintaining it.

@heifner
Copy link
Contributor Author

heifner commented Aug 27, 2019

No need to block this PR as cpu billing is subjective however, this is a "change of policy" of sorts and we should consider documenting it somewhere or maintaining the old "policy"

It was not always this way. In 1.5.x it "correctly" calculated it. Although we have had other issues in actually using the correct calculation in the past. In short we have had a number of issues with correctly calculating and using the value. So much so that documentation of any given past policy would be difficult. For example: #6871.

@heifner heifner mentioned this pull request Aug 28, 2019
3 tasks
@heifner heifner merged commit 77e0678 into develop Aug 28, 2019
@heifner heifner deleted the sig-cpu-usage branch August 28, 2019 21:17
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.

2 participants