New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ExtractWinInetValues function #4

Merged
merged 2 commits into from Mar 29, 2018

Conversation

Projects
None yet
2 participants
@amiyagupta
Copy link

amiyagupta commented Mar 29, 2018

Following the upgrade to the TraceEvent library, the ExtractWinInetValues function which did some custom parsing of various WinInet events appears to no longer be necessary. This is a very significant performance win because the objects being passed in are dynamically typed. It is the most expensive function in the exe, typically taking 50-60% of CPU time (measured after upgrading to the latest TraceEvent library).

In a separate branch (https://github.com/amiyagupta/wpt-etw/tree/examine-extract-wininet-values) I've added logging that compares fields before and after ExtractWinInetValues updates them and prints out stats before exiting. In the vast majority of cases the existing field is overwritten by an identical string. In the few cases where the before and after strings do not match, the original string is more correct than the one it is being replaced with.

All the cases where the before and after strings do not match fall into two categories:

  1. Extra trailing characters in ServerName field in WININET_TCP_CONNECTION/Start
  2. Extra leading characters in the AddressList field update in WININET_DNS_QUERY/Stop
TcpConnectionStart ServerName modified from
nav.smartscreen.microsoft.com
to
nav.smartscreen.microsoft.com\

DnsQueryStop AddressList modified from
52.9.194.244;54.153.119.33;52.52.19.88;13.56.126.150;
to
5 52.9.194.244;54.153.119.33;52.52.19.88;13.56.126.150; 

Some example statistics I gathered while I had the version with logging running and browsing around a large number of popular websites:

HeaderOverwrites: 0, HeaderOverWritesWasted: 0
DnsQueryStartOverwrites: 459, DnsQueryStartOverwritesWasted: 459
DnsQueryStopOverwrites: 454, DnsQueryStopOverwritesWasted: 454
DnsQueryStopAddressOverwrites: 454, DnsQueryStopAddressOverwritesWasted: 441
TcpConnectionStartOverwrites: 821, TcpConnectionStartOverwritesWasted: 779
SendRequestStopOverwrites: 2109, SendRequestStopOverwritesWasted: 2109
ConnectStopLocalAddressOverwrites: 0, ConnectStopLocalAddressOverwritesWasted: 0
ConnectStopRemoteAddressOverwrites: 0, ConnectStopRemoteAddressOverwritesWasted: 0
@amiyagupta

This comment has been minimized.

Copy link

amiyagupta commented Mar 29, 2018

Should mention I've also verified that network details like request/response headers, DNS lookup time, IP address fields continue to work with this change.

@pmeenan

This comment has been minimized.

Copy link
Contributor

pmeenan commented Mar 29, 2018

Awesome! That was always an ugly hack and I'm thrilled it is gone.

@pmeenan pmeenan merged commit c8b887a into WPO-Foundation:master Mar 29, 2018

@amiyagupta amiyagupta deleted the amiyagupta:disable-extract-wininet-values branch Mar 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment