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

add accurate checktime timer for macOS #7449

Merged
merged 7 commits into from
Jul 2, 2019
Merged

Conversation

spoonincode
Copy link
Contributor

Change Description

pre-1.4 nodeos would frequently poll the system time during contract execution (to ensure a contract wasn’t executing beyond its allowed time). For most platforms this isn’t a syscall (see vDSO) but it still was a significant overhead.

1.4 (via #5799) introduced a checktime timer where a kernel timer — via BSD interval timer interface — would be scheduled instead. This allows nodeos to skip polling of system time until the timer expires. There are a few problems with the current implementation:

  1. While macOS supports the system interval timer API, it heuristically allows potentially considerable leeway in the expiration. For example, a 30ms timer could expire 35+ms instead. This is the reason for the "timer inaccurate" messages when starting nodes on macOS: it detects significant error in the timer and decides not to use it
  2. The code was POSIX and interferes with win32 builds
  3. There is copy-pasta with how the "calibration" (accuracy check) is performed
  4. Only a single timer can be used because there is only a single BSD interval timer. In practice this isn’t a huge deal at the moment.

This PR addresses all of these issues via

  1. On macOS the timer now uses the kevent timer interface. This allows us to specify the timer expiration is “critical” and macOS gives us considerably more accuracy. What use to be Using polled checktime; deadline timer too inaccurate: min:5us max:2540us mean:507us stddev:716us on my MBP is now Using 54us deadline timer for checktime: min:3us max:112us mean:22us stddev:16us.
  2. cmake does a simple check on platform and disables any timer code if the platform doesn’t support it. This means those platforms will fall back to a poll-only checktime like pre 1.4
  3. "Calibration"/accuracy detection is now done in a way that reuses the existing checktime_timer object instead of reimplementing its logic. However, a critical goal here was to perform this calibration (which can take a few hundred ms) earlier in nodeos startup instead of trying to do it lazily latter on like during construction of something important. Hence the use of a static global object here.
  4. the kevent timer interface support multiple timers so it was reasonable to refactor the Linux code to use the POSIX timer API to support multiple timers. A side effect of this though is that creation & destruction of a deadline_timer is no longer free. I’ve moved the deadline_timer to controller to prevent it from being created & destroyed each time a transaction context is created. Not really sure this is the best place for it but it's reasonably appropriate at the moment.

Resolves long standing #6693

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

Add a kevent based timer for accurate checktime timer on macOS. Also, refactor the code such that the appropriate platform specific timer code is used. On platforms without an implementation fall back to poll mode. Also, refactor how the time accuracy is verified ("calibrated") so that it is not a copy paste job and can use the checktime_timer object itself.
});
}

//a kludge until something better: print stats only after calibration completes
Copy link
Contributor

Choose a reason for hiding this comment

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

Cant this safely be done at the beginning of something like controller::startup at that point calibration should be done (it seems synchronous?) and fc::logging should be initialized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that doesn't seem to pose any risks from what I could tell -- is there anything that will be disturbed by the first global instance of controller taking a few hundred milliseconds to start up?

@spoonincode spoonincode requested a review from b1bart July 1, 2019 21:31
@spoonincode spoonincode merged commit 6b58624 into develop Jul 2, 2019
@spoonincode spoonincode deleted the mac_checktime_timer branch July 2, 2019 15:58
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