Repository navigation
System.Net.WebProxy Bypass regex list reconstructed for every call to IsBypassed #73703
Description
Description
System.Net.WebProxy takes a list of strings for Uris that should bypass the proxy.
This list of strings is compiled into a list of Regex by the method UpdateRegexList
This method is called in the constructor and in the BypassList property setter.
This is normal.
However, this method is also called in IsMatchInBypassList, which is called by IsBypassed
This feels wrong. I don't see why it needs to systematically refresh the regex list in this call.
It re-allocates the array and all the regex, for no reason (as far as I can see anyway).
Regression?
This behavior seems to be copy-pasted from the .Net Framework code, which is also wrong.
https://referencesource.microsoft.com/#system/net/system/Net/webproxy.cs
Analysis
The following code shows a significant difference between the proxy1 et proxy2 execution time
(typically ~230ms vs ~2100ms)
Since the call to IsBypassed returns true for the first regex, the length of the Bypasslist should not matter.
Uri uri = new Uri("https://a.com/");
System.Diagnostics.Stopwatch sw = new System.Diagnostics.Stopwatch();
bool r1 = true;
System.Net.WebProxy proxy1 = new System.Net.WebProxy("http://localhost:9999", true, new string[] { "a" });
sw.Restart();
for (int i = 0; i < 100000; i++) {
r1 &= proxy1.IsBypassed(uri);
}
sw.Stop();
Console.WriteLine($"Proxy1 : {sw.ElapsedMilliseconds}ms ({r1})");
bool r2 = true;
System.Net.WebProxy proxy2 = new System.Net.WebProxy("http://localhost:9999", true, new string[] { "a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z" });
sw.Restart();
for (int i = 0; i < 100000; i++) {
r2 &= proxy2.IsBypassed(uri);
}
sw.Stop();
Console.WriteLine($"Proxy2 : {sw.ElapsedMilliseconds}ms ({r2})");
Activity
MihaZupan commentedon Aug 10, 2022
I also don't see any reason to rebuild the list in this case. Doing so looks like just a waste of compute.
Should be a simple change - marking as up for grabs. @onehourlate would you be interested in contributing a fix for this?
campersau commentedon Aug 10, 2022
It looks like the underlying
_bypassListis publicly exposed asBypassArrayListwhich means that it could be modified from the outside:runtime/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs
Line 85 in de32c44
MihaZupan commentedon Aug 10, 2022
You're right... yay legacy APIs :)
We can still do a lot better here than creating all Regex objects on every request though.
At a minimum have a flag checking whether that property was ever accessed and skip
UpdateRegexListon the hot path.onehourlate commentedon Aug 10, 2022
Or even subclass ArrayList for the BypassArrayList with something that keeps a dirty flag.
Since all methods of an ArrayList that modify the list are virtual, that's easily done.
(what's a bit frustrating is that the ArrayList does that for us already, via the version, but it's private)
And then check the dirty flag to see if we need to rebuild the regex list.
onehourlate commentedon Aug 11, 2022
Made a PR (#73803), if it helps
(I just skimmed the guidelines, so sorry if I missed something ...)
Changes in the BypassArrayList are tracked by a Dirty flag
RegexList is only updated if Dirty flag is set
Added a few unit tests (one unit test uses reflexion : not sure if it's "allowed")
System.Net.WebProxy Bypass regex list optimization