Attached is the current patch in case it gets approved for an interim merge.
On Tue, Jan 13, 2015 at 11:19 AM, Kinkie <[email protected]> wrote: > On Tue, Jan 13, 2015 at 3:09 AM, Amos Jeffries <[email protected]> wrote: >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 >> >> On 13/01/2015 8:51 a.m., Kinkie wrote: >>> Hello, the attached patch changes some users of splay to std::set. >>> The aim is to get more predictable (if not necessarily better) >>> performance and leverage a cleaner API resulting in more readable >>> code. >>> >>> This is a merge snapshot of branch lp:~squid/squid/replace-splay >>> revno 13835; I will continue working on the branch to migrate more >>> users. >>> >>> The touched classes have been build- and run-tested. >>> >> >> >> in src/acl/UserData.h, src/acl/Eui64.h, etc >> * still need to include splay.h in these .h files? >> perhapse it can be reduced down to the relevant .cc or dropped entirely. > > dropped. > >> in src/acl/Eui64.cc: >> * ACLEui64::parse() can be optimized a bit further: >> >> while (const char *t = ConfigParser::strtokFile()) { >> if (Eui::Eui64 *q = aclParseEuiData(t)) { >> ... what used to follow the if >> xfree(q); >> } > > done, although it is not a performance-critical path. > >> } >> >> * ACLEui64::dump() buf variable can remain as static. It's only used >> until SBuf data-copies, no need to re-allocate on each loop. > > Ok > >> - also while changing, if possible use SZ_EUI64_BUF otherwise >> document why magic number is 48 used. (EUI uses 3 readable characters >> to represent each octet of the number - buf is approx. double the >> required size). > > SZ_EUI64_BUF is 8; I understand that's for the binary representation, > not textual. > I kept the buffer as it was, after all it's not huge anyway. > >> >> >> in src/acl/UserData.cc: >> * #include SbufAlgos.h before util.h > > Done. > >> * for ctor/dtor like ACLUserData::~ACLUserData() which have become >> NOP, please inline them in the .h where possible. That allows some >> more compiler optimizations. > > Is that even true when they are virtual? But sure, it means fewer > lines of code so that's OK. > >> * is cend() usable instead of end() for the find test in the function >> above ACLUserData::dump() [ chunk @55, I think its probably the end of >> match()] > > Using const iterators in the copied-from container. > >> * why is CaseInsensitveSBufCompare() not defined in SBufAlgos.h for >> re-use ? > > Should I do that as part of this branch or merge separately? -- Francesco
replace-splay-v2.patch
Description: Binary data
_______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
