On Mon, Apr 28, 2014 at 10:46 AM, Jonathan Wakely <jwak...@redhat.com> wrote: > This change splits _Executor::_M_main() into two overloaded > _M_main_dispatch() functions, choosing which to run based on the > __dfs_mode template parameter. > > I think this gives a (very) small improvement in compilation time when > using regexes. > > Splitting _M_main() allows the _M_match_queue and _M_visited members > (which are only used in BFS mode) to be replaced with an instantiation > of the new _States class template. _States<__dfs> only contains the > start state (so that it's not empty and doesn't waste space) but > _States<__bfs> contains the start state and also the match queue and > visited list.
This is great! I keep worrying about the ugly all-in-one _Executor class; I've tried to specialize two versions of it, but that introduced much duplicated code. Your abstract is nice! > As the visited list never changes size after construction I changed > its type from unique_ptr<vector<bool>> to unique_ptr<bool[]>, which > avoids an indirection, although now that that member is only present > when actually required it could just be vector<bool>. An array of bool > is simpler to access, but takes more heap memory. vector<bool> uses > less space but the compiler has to do all the masking to address > individual bits. What about bitset? Anyway we should get rid of vector<bool>. > I also changed this range-based for loop in _M_main (now in > _M_main_dispatch(__bfs)) to make __task a reference, avoiding one > copy, and to move from __task.second, avoiding a second copy: > > auto __old_queue = std::move(_M_states._M_match_queue); > for (auto& __task : __old_queue) > { > _M_cur_results = std::move(__task.second); > _M_dfs<__match_mode>(__task.first); > } I didn't know why I've written the code that copys a lot. Thanks. -- Regards, Tim Shen