Yield twice instead of once (#2803)

Fix #2786

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

bin/light-base/src/json_rpc_service.rs

@@ -595,7 +595,7 @@ impl<TPlat: Platform> Background<TPlat> {
 
                         // We yield once between each request in order to politely let other tasks
                         // do some work and not monopolize the CPU.
-                        crate::util::yield_once().await;
+                        crate::util::yield_twice().await;
                     }
                 }
                 .boxed(),

bin/light-base/src/runtime_service.rs

@@ -2017,7 +2017,7 @@ impl SuccessfulRuntime {
         heap_pages: &Option<Vec<u8>>,
     ) -> Result<Self, RuntimeError> {
         // Since compiling the runtime is a CPU-intensive operation, we yield once before.
-        crate::util::yield_once().await;
+        crate::util::yield_twice().await;
 
         // Parameters for `HostVmPrototype::new`.
         let module = code.as_ref().ok_or(RuntimeError::CodeNotFound)?;

bin/light-base/src/sync_service/standalone.rs

@@ -135,7 +135,7 @@ pub(super) async fn start_standalone_chain<TPlat: Platform>(
                 // in a row would prevent all the other tasks in the background from running.
                 // In order to provide a better granularity, we force a yield after each
                 // verification.
-                crate::util::yield_once().await;
+                crate::util::yield_twice().await;
             }
 
             queue_empty

bin/light-base/src/util.rs

@@ -16,14 +16,25 @@
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 /// Use in an asynchronous context to interrupt the current task execution and schedule it back.
+/// Twice.
 ///
 /// This function is useful in order to guarantee a fine granularity of tasks execution time in
 /// situations where a CPU-heavy task is being performed.
-pub async fn yield_once() {
-    let mut pending = true;
+///
+/// We do not yield once, but twice.
+/// The reason is that, at the time of writing, `FuturesUnordered` yields to the outside after
+/// one of its futures has yielded twice. We use `FuturesUnordered` in the Wasm node.
+/// Yielding to the outside is important in the context of the browser node because it gives
+/// time to the browser to run its own events loop.
+/// See <https://github.com/rust-lang/futures-rs/blob/7a98cf0bbeb397dcfaf5f020b371ab9e836d33d4/futures-util/src/stream/futures_unordered/mod.rs#L531>
+/// See <https://github.com/rust-lang/futures-rs/issues/2053> for a discussion about a proper
+/// solution.
+// TODO: this is a complete hack ^
+pub async fn yield_twice() {
+    let mut num_pending_remain = 2;
     futures::future::poll_fn(move |cx| {
-        if pending {
-            pending = false;
+        if num_pending_remain > 0 {
+            num_pending_remain -= 1;
             cx.waker().wake_by_ref();
             core::task::Poll::Pending
         } else {

bin/wasm-node/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Fixed
+
+- CPU-heavy operations such as verifying finality proofs or compiling the runtime will now better respect the CPU rate limit. ([#2803](https://github.com/paritytech/smoldot/pull/2803))
+
 ## 0.7.0 - 2022-09-28
 
 ### Removed