Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: src/plugin/ActiveQueue.h

Issue 29323611: Issue #1234, #2058 - Rewrite log facility, improving thread implementation
Left Patch Set: rebase to current tip Created Jan. 5, 2016, 2:52 p.m.
Right Patch Set: rebase only Created July 27, 2016, 9:11 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « adblockplus.gyp ('k') | src/plugin/Config.h » ('j') | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-2015 Eyeo GmbH 3 * Copyright (C) 2006-2016 Eyeo GmbH
Oleksandr 2016/01/28 10:15:34 Nit: Here and further sadly we have already update
Eric 2016/02/04 21:01:44 Done.
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12 * GNU General Public License for more details. 12 * GNU General Public License for more details.
13 * 13 *
14 * You should have received a copy of the GNU General Public License 14 * You should have received a copy of the GNU General Public License
15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>. 15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
16 */ 16 */
17 17
18 #include <thread> 18 #include <thread>
19 #include <mutex> 19 #include <mutex>
20 #include <condition_variable> 20 #include <condition_variable>
21 #include <deque> 21 #include <deque>
22 #include "Placeholder.h" 22 #include "Placeholder.h"
23 23
24 /** 24 /**
25 * A synchronized FIFO queue with thread notice on receipt. 25 * A synchronized FIFO queue with thread notice on receipt.
26 * 26 *
27 * \tparam T 27 * \tparam T
28 * Class of the elements of the queue. 28 * Class of the elements of the queue.
29 * It must have a move constructor. 29 * It must have a move constructor.
30 * \tparam F
31 * Functor type of object to receive notice on Insert().
Oleksandr 2016/01/28 10:15:34 Nit: I guess this parameter was removed at some po
Eric 2016/02/04 21:01:43 Yes. As I recall, I originally only had one class.
32 */ 30 */
33 template<class T> 31 template<class T>
34 class MessageQueue 32 class MessageQueue
35 { 33 {
36 std::deque<T> queue; 34 std::deque<T> queue;
37 typedef std::lock_guard<std::mutex> SentryType; 35 typedef std::lock_guard<std::mutex> SentryType;
38 typedef std::unique_lock<std::mutex> UniqueLockType; 36 typedef std::unique_lock<std::mutex> UniqueLockType;
39 std::mutex mutex; 37 std::mutex mutex;
40 std::condition_variable cv; 38 std::condition_variable cv;
41 public: 39 public:
42 MessageQueue() {} // = default 40 MessageQueue() {} // = default
43 41
44 /* 42 /*
45 * This class does not have any responsibility with regard to the elements of its queue. 43 * This class does not have any responsibility with regard to the elements of its queue.
46 * An instance may be destroyed with elements still in the queue. 44 * An instance may be destroyed with elements still in the queue.
47 */ 45 */
48 ~MessageQueue() {} // = default; 46 ~MessageQueue() {} // = default;
49 47
50 /** 48 /**
51 * Insert an element, copy constructor version. 49 * Insert an element, copy semantics version.
Oleksandr 2016/01/28 10:15:34 Nit: this is not a constructor. I think "lvalue ve
Eric 2016/02/04 21:01:44 OK. I'll use "copy semantics".
52 */ 50 */
53 void Insert(const T& t) 51 void Insert(const T& t)
54 { 52 {
55 { 53 SentryType sentry(mutex);
Oleksandr 2016/01/28 10:15:35 Just to reiterate Sergei's point - the extra scope
Eric 2016/02/04 21:01:44 Done.
56 SentryType sentry(mutex); 54 queue.push_back(t);
57 queue.push_back(t); 55 cv.notify_one();
58 cv.notify_one(); 56 }
59 } 57
60 } 58 /**
61 59 * Insert an element, move semantics version.
62 /**
63 * Insert an element, move constructor version.
64 */ 60 */
65 void Insert(T&& t) 61 void Insert(T&& t)
66 { 62 {
67 { 63 SentryType sentry(mutex);
68 SentryType sentry(mutex); 64 queue.push_back(std::move(t));
69 queue.push_back(std::move(t)); 65 cv.notify_one();
70 cv.notify_one();
71 }
72 } 66 }
73 67
74 /** 68 /**
75 * Wake up anything waiting on the condition variable. 69 * Wake up anything waiting on the condition variable.
76 * 70 *
77 * \tparam Function 71 * \tparam Function
78 * Functor type for argument 72 * Functor type for argument
79 * \param f 73 * \param f
80 * Functor to execute _inside_ the protection of the queue's mutex 74 * Functor to execute _inside_ the protection of the queue's mutex
81 */ 75 */
82 template<typename Function> 76 template<typename Function>
83 void Rouse(Function& f) 77 void Rouse(Function& f)
84 { 78 {
85 SentryType sentry(mutex); 79 SentryType sentry(mutex);
86 f(); 80 f();
87 cv.notify_one(); 81 cv.notify_one();
88 } 82 }
89 83
90 /** 84 /**
91 * Remove and Test. 85 * Test, Remove, and Execute
92 * Test that the queue is not empty. 86 *
93 * If so, remove the next element from the consumer end of the queue and pass it to the argument function 87 * If the queue is not empty, remove the next element from the consumer end
Oleksandr 2016/01/28 10:15:34 Nit: I think there are too many comments in this f
Eric 2016/02/04 21:01:43 Reworded.
88 * of the queue and execute the argument functor on it.
94 * 89 *
95 * \param f 90 * \param f
96 * If return value is true, a functor to which to pass the element at the co nsumer end of the queue. 91 * A functor to which to pass the element at the consumer end of the queue.
97 * This functor is executed _outside_ the protection of the queue's mutex. 92 * This functor is executed _outside_ the protection of the queue's mutex.
98 * \return 93 * \return
99 * True if and only if an element was removed from the front of the queue. 94 * True if and only if an element was removed from the front of the queue.
100 */ 95 */
101 template<typename Function> 96 template<typename Function>
102 bool Remove(Function& f) 97 bool Remove(Function& f)
103 { 98 {
104 /* 99 /*
105 * We need to remove an element atomically from the queue, 100 * We need to remove an element atomically from the queue,
106 * so we need to construct that element _inside_ the protection of the mut ex, 101 * so we need to construct that element _inside_ the protection of the mut ex,
107 * On the other hand, we need to execute the function _outside_ the protecti on of the queue, 102 * On the other hand, we need to execute the function _outside_ the protecti on
108 * because otherwise performance may suffer. 103 * of the queue, because otherwise performance may suffer.
109 * Hence we use 'Placeholder' so that we can declare a variable to hold the removed element 104 * Hence we use 'Placeholder' so that we can declare a variable to hold the
110 * without requiring it to have a default constructor or assignment operat ors. 105 * removed element without requiring it to have a default constructor or
106 * assignment operators.
111 */ 107 */
112 Placeholder<T> x; 108 Placeholder<T> x;
113 { 109 {
114 SentryType sentry(mutex); 110 SentryType sentry(mutex);
115 if (queue.empty()) 111 if (queue.empty())
116 return false; 112 return false;
117 x.Construct(std::move(queue.front())); // only require move-constructib ility 113 x.Construct(std::move(queue.front())); // only require move-constructib ility
118 queue.pop_front(); 114 queue.pop_front();
119 } 115 }
120 f(std::move(x.Object())); 116 f(std::move(x.Object()));
(...skipping 155 matching lines...) Expand 10 before | Expand all | Expand 10 after
276 } 272 }
277 273
278 /** 274 /**
279 * Insert 275 * Insert
280 */ 276 */
281 void Insert(T&& t) 277 void Insert(T&& t)
282 { 278 {
283 queue.Insert(std::move(t)); 279 queue.Insert(std::move(t));
284 } 280 }
285 }; 281 };
LEFTRIGHT

Powered by Google App Engine
This is Rietveld