This is the mail archive of the
mailing list for the GCC project.
Re: [patch 3/N] std::regex refactoring - _Executor DFS / BFS
- From: Tim Shen <timshen91 at gmail dot com>
- To: Jonathan Wakely <jwakely at redhat dot com>
- Cc: "libstdc++" <libstdc++ at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 28 Apr 2014 11:40:48 -0400
- Subject: Re: [patch 3/N] std::regex refactoring - _Executor DFS / BFS
- Authentication-results: sourceware.org; auth=none
- References: <20140428124003 dot GW928 at redhat dot com> <20140428144659 dot GY928 at redhat dot com>
On Mon, Apr 28, 2014 at 10:46 AM, Jonathan Wakely <email@example.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);
I didn't know why I've written the code that copys a lot. Thanks.