> On May 4, 2016, 10:53 p.m., Michael Park wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2240-2270
> > <https://reviews.apache.org/r/46395/diff/2/?file=1358642#file1358642line2240>
> >
> >     Consider this if you would to preserve the guarantee:
> >     
> >     ```cpp
> >       struct
> >       {
> >         void operator()() const
> >         {
> >           do {
> >             ProcessBase* process = process_manager->dequeue();
> >             if (process == NULL) {
> >               Gate::state_t old = gate->approach();
> >               process = process_manager->dequeue();
> >               if (process == NULL) {
> >                 if (joining_threads.load()) {
> >                   break;
> >                 }
> >                 gate->arrive(old);
> >                 // Wait
> >                 // at
> >                 // gate
> >                 // if
> >                 // idle.
> >                 continue;
> >               } else {
> >                 gate->leave();
> >               }
> >             }
> >             process_manager->resume(process);
> >           } while (true);
> >         }
> >     
> >         // We hold a constant reference to `joining_threads` to make it 
> > clear that this
> >         // value is only being tested (read), and not manipulated.
> >         const std::atomic_bool& joining_threads;
> >       } worker{joining_threads};
> >     
> >       // Create processing threads.
> >       for (long i = 0; i < num_worker_threads; i++) {
> >         // Retain the thread handles so that we can join when shutting down.
> >         threads.emplace_back(new std::thread(worker));
> >       }
> >     ```
> 
> Alex Clemmer wrote:
>     I like this code, but I would feel odd submitting a review that is almost 
> entirely your code, and then taking credit for it in the commit log. It might 
> make sense to do a follow up?

Haha, don't worry about it. If you like to suggestion, please update this 
review and we'll commit it!

Just a note, it looks like I messed up the formatting in the suggestion:

```
            gate->arrive(old);
            // Wait
            // at
            // gate
            // if
            // idle.
```

should be:

```
            gate->arrive(old); // Wait at gate if idle.
```


- Michael


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46395/#review131755
-----------------------------------------------------------


On May 5, 2016, 3:55 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46395/
> -----------------------------------------------------------
> 
> (Updated May 5, 2016, 3:55 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3637
>     https://issues.apache.org/jira/browse/MESOS-3637
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Windows: Removed `std::bind` from `process.cpp` to build on Windows.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp 
> 8727eb202e9699f0ac3c95788257cf1a22b0da7b 
> 
> Diff: https://reviews.apache.org/r/46395/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Reply via email to