On 2013/11/19 13:12, Denis Krjuchkov <de...@crazydev.net> wrote: > I'm trying to implement new event loop for MPD without GLib (or any > other library). > It's borrowed from existing epoll-based event loop. > I only abstracted API differences behind general inteface (PollGroup class). > Currently there are 3 backends: poll(), epoll() and Windows variant of > select(). > Code is poorly tested and has no comments at the moment, consider this a > prototype.
My few cents: - you reordered code in event/Loop.cxx, which makes reading the diff more difficult than necessary - the PollGroup/PollGroupImpl indirection adds unnecessary runtime overhead, without any advantage - why did you remove class EPollFD? Instead, PollGroupEPoll contains this abstraction code, plus the PollGroupImpl code. - why add overhead by storing the events in EventQueue? My implementation could work without this storage class. Your commit adds this overhead even to the epoll implementation. - why implement the SocketSet copy constructor? - you removed all the GLib code in that one commit, and added all new (untested) implementations; much of the commit deals with removing GLib specific code The last one is unfortunate, because it means we have to do the full switch in one big step. It would be better to have incremental patches which add one (optional) new implementation at a time; now we have to be sure all of them are mature at the same time. At the end, when we are portable across all platforms, we could remove the GLib implementation. ------------------------------------------------------------------------------ Shape the Mobile Experience: Free Subscription Software experts and developers: Be at the forefront of tech innovation. Intel(R) Software Adrenaline delivers strategic insight and game-changing conversations that shape the rapidly evolving mobile landscape. Sign up now. http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk _______________________________________________ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
