Address nits

rust-lang · cramertj · Mar 11, 2019 · Apr 24, 2018 · May 30, 2018 · Nov 9, 2018
commit bb242c8cc6ee2f873e6f95d1bf02cedf4cded468
diff --git a/text/0000-futures.md b/text/0000-futures.md
@@ -178,11 +178,33 @@ pub trait Wake: Send + Sync {
    ///
    /// Executors generally maintain a queue of "ready" tasks; `wake_local` should place
    /// the associated task onto this queue.
-
    unsafe fn wake_local(self: &Arc<Self>)
 }
 ```

+To see how this might be used in practice, here's a simple example sketch:
+
+```rust
+struct ExecutorInner {
+    sync_ready_queue: SynchronousQueue,
+    optimized_queue: UnsafeCell<Vec<Task>>,
+}
+
+struct Task {
+    future: ...,
+    executor: Arc<ExecutorInner>,
+}
+
+impl Wake for Task {
+    fn wake(self: &Arc<Self>) {
+        self.executor.sync_ready_queue.push(self.clone());
+    }
+    unsafe fn wake_local(self: &Arc<Self>) {
+        (&mut *self.executor.optimized_queue.get()).push(self.clone())
+    }
+}
+```
+
 The use of `&Arc<Self>` rather than just `&self` makes it possible to work directly with
 the trait object for `Wake`, including cloning it. With `UnsafeWake` below, we'll see
 an API with greater flexibility for the cases where `Arc` is problematic.
@@ -208,14 +230,15 @@ impl Waker {
 }

 /// A `LocalWaker` is a handle for waking up a task by notifying its executor that it is ready to be run.
-
+///
 /// This is similar to the `Waker` type, but cannot be sent across threads. Task executors can use this type to implement more optimized singlethreaded wakeup behavior.
-
 impl LocalWaker {
    /// Wake up the task associated with this `LocalWaker`.
    pub fn wake(&self);
 }

+/// You can upgrade to a sendable `Waker` at zero cost, but waking through a `Waker` is more expensive
+/// due to synchronization.
 impl From<LocalWaker> for Waker  { .. }
 ```
-Original file line number
+Diff line change
@@ -178,11 +178,33 @@ pub trait Wake: Send + Sync {
         ///
         /// Executors generally maintain a queue of "ready" tasks; `wake_local` should place
         /// the associated task onto this queue.
         unsafe fn wake_local(self: &Arc<Self>)
     }
     ```
+    To see how this might be used in practice, here's a simple example sketch:
+    ```rust
+    struct ExecutorInner {
+        sync_ready_queue: SynchronousQueue,
+        optimized_queue: UnsafeCell<Vec<Task>>,
+    }
+    struct Task {
+        future: ...,
+        executor: Arc<ExecutorInner>,
+    }
+    impl Wake for Task {
+        fn wake(self: &Arc<Self>) {
+            self.executor.sync_ready_queue.push(self.clone());
+        }
+        unsafe fn wake_local(self: &Arc<Self>) {
+            (&mut *self.executor.optimized_queue.get()).push(self.clone())
+        }
+    }
+    ```
     The use of `&Arc<Self>` rather than just `&self` makes it possible to work directly with
     the trait object for `Wake`, including cloning it. With `UnsafeWake` below, we'll see
     an API with greater flexibility for the cases where `Arc` is problematic.
                 Outdated
          
        

                


  
    
        
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
  Member


      

  

  
    
      

      
            RalfJung
  

      

      

      


        Nov 14, 2018


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

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

    
      
      


  


  
    
      I first thought this would refer to the example just above, but there is no trait object there, is there? An example for what this remark refers to would be nice.
I fist thought one could just add Clone to the supertraits, but I guess that would destroy object safety.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



    
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
  Member


      

  

  
    
      

      
            cramertj
  

      

      

      


        Nov 15, 2018


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

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

    
      
      


  


  
    
      LocalWaker and Waker are themselves trait objects. Both work out to something like *const UnsafeWake. Implementing the Wake trait gives you an implementation of UnsafeWake for Arc<YourType>.
This API can be awkward to work with in some edge cases though where you want to change the vtable without changing the underlying pointed to type. @aturon is working on a modification of the proposal, and I've suggested considering that we use a more C-like manual vtable instead. This would allow us more flexibility, make type-punning-to-change-the-vtable unnecessary, and removes the hairy questions around whether or not *const UnsafeWake must have a valid vtable.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
@@ -208,14 +230,15 @@ impl Waker {
     }
     /// A `LocalWaker` is a handle for waking up a task by notifying its executor that it is ready to be run.
+    ///
     /// This is similar to the `Waker` type, but cannot be sent across threads. Task executors can use this type to implement more optimized singlethreaded wakeup behavior.
     impl LocalWaker {
         /// Wake up the task associated with this `LocalWaker`.
         pub fn wake(&self);
     }
+    /// You can upgrade to a sendable `Waker` at zero cost, but waking through a `Waker` is more expensive
+    /// due to synchronization.
     impl From<LocalWaker> for Waker  { .. }
     ```
-          Expand Down