On 11/13/2016 05:11 PM, Kinkie wrote: > the attached patch moves away from hand-rolling a c-string onto > joining a SBufList for optimizing regexes in RegexData.cc.
> You can find attached as a test case the output of squidclient > mgr:config taken on trunk and on the submitted code. It is slightly > different in that the new code allows for a longer optimized regex. I see the following before/after difference: -acl bigacl dstdom_regex ... (pattern84) (pattern86) ... +acl bigacl dstdom_regex ... (pattern84)|(pattern85) ... Does changing space into "|" result in one internal regex instead of two? And that is the optimization you have implemented? How is that related to SBufs (i.e., why could not old c-strings accommodate longer regexes)? And why is there no pattern85 in the "before" test?! > The code is expected to cause a small performance regression on > parse and reconfigure due to extra data copies. The regression is > negligible: the attached test cases perform "squid -k parse" in 60 > msec in trunk and 62 on the branch on a warm cache on my i5 macbook > air. A 3% performance regression is not necessarily negligible. Have you tested with more ACL lines (not larger individual ACL lines) that take a few seconds to load (as opposed to 60ms) ? If not, please do unless that test would be irrelevant somehow. And what is the expected performance improvement from having fewer longer regexes? > [RFC] This should be a different thread IMHO. > The code managing case-insensitivity flags is IMO quite complicated > and not really intuitive: it switches between case-insensitive and > case-sensitive each time it meets a -i flag. It does?! Wow! The documentation says that +i switches back to case-sensitive rather than repeating -i values. If Squid does something else here, that would be a pretty big bug IMO! > I would like to change > the behaviour so that a -i flag makes the whole acl case-insensitive; > this seems to me consistent with the behavior documented in squid.conf > which does not document the case of multiple -i flags in a single > regex-type ACL. I'd like feedback on this proposal. I am not 100% sure what you mean by "whole acl" exactly, but please see the "Where do ACL flags belong?" RFC[1] for a comprehensive coverage of a similar problem and the agreed solution. The initial discussion was mostly noise, but things started to make sense around [2]. Your -i is mentioned frequently, including the last(?) email on that thread[3]. AFAICT, the agreed solutions have not been implemented yet so if you have the cycles to work on this, you may be able to address all the related problems at once and make things consistent across all options of all ACLs. You should probably post a revised RFC (summarizing all known issues and corresponding changes) if you are going to do that. [1] http://lists.squid-cache.org/pipermail/squid-dev/2015-December/004034.html [2] http://lists.squid-cache.org/pipermail/squid-dev/2015-December/004044.html [3] http://lists.squid-cache.org/pipermail/squid-dev/2015-December/004096.html Thank you, Alex. _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev