[PATCH v3 0/6] MR78: NSI and GetAdaptersInfo() performance improvements
This is inspired by Street Fighter V game which performance has regressed in multiplayer mode since iphlpapi reimplementation on top of NSI. Speficically, the game calls GetAdapatersInfo() very often which turnaround time increased dramatically. While I was checking the effect against GetAdapatersInfo() only I hope the patchset might improve the performance of some other iphlpapi functions as well. Here is the patch I used as a top level measure of the effect: https://gist.github.com/gofman/c08e58f15105d60b4576c9ae25bcec9d The table below lists the times of GetAdaptersInfo() in ms measured with this test (this is the time for GetAdapatersInfo(NULL, ...) for estimating the buffer length plus actual data query call. The major difference between Machine 1 and Machine 2 is most likely in the number of ip routing table entries (8 on "Machine 2" and 4 on "Machine 1"). Windows has less interfaces and routes configured so it is probably not suitable for back to back performance comparison this way although the datapoint may still be interesting for very rough comparison. The baseline here is the performance before iphlpapi redesign (which I measured on Proton 6.3). There are maybe more opportunities for optimization but after this patchset GetAdapatersInfo() performance is slightly improved over baseline so I stopped here for now. ``` Machine 1 Machine 2 Windows 0.7 Proton 6.3 2.7 8.7 Current Wine 12.2 99.8 After patch 1 10.0 59.2 After patch 4 5.5 27.2 After patch 5 3.6 20.2 After patch 6 2.4 7.6 ``` -- v3: iphlpapi: Don't request unused dynamic interface data. nsi: Allocate a small buffer at once in NsiAllocateAndGetTable(). nsiproxy.sys: Update interface table only if LUID is not found in convert_unix_name_to_luid(). nsiproxy.sys: Update interface table only if LUID is not found in convert_luid_to_unix_name(). https://gitlab.winehq.org/wine/wine/-/merge_requests/78
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/nsiproxy.sys/ndis.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/dlls/nsiproxy.sys/ndis.c b/dlls/nsiproxy.sys/ndis.c index ba4933ff16b..a3a10148109 100644 --- a/dlls/nsiproxy.sys/ndis.c +++ b/dlls/nsiproxy.sys/ndis.c @@ -495,9 +495,11 @@ static NTSTATUS ifinfo_get_all_parameters( const void *key, UINT key_size, void pthread_mutex_lock( &if_list_lock ); - update_if_table(); - - entry = find_entry_from_luid( (const NET_LUID *)key ); + if (!(entry = find_entry_from_luid( (const NET_LUID *)key ))) + { + update_if_table(); + entry = find_entry_from_luid( (const NET_LUID *)key ); + } if (entry) { ifinfo_fill_entry( entry, NULL, rw_data, dynamic_data, static_data ); @@ -557,9 +559,11 @@ static NTSTATUS ifinfo_get_parameter( const void *key, UINT key_size, UINT param pthread_mutex_lock( &if_list_lock ); - update_if_table(); - - entry = find_entry_from_luid( (const NET_LUID *)key ); + if (!(entry = find_entry_from_luid( (const NET_LUID *)key ))) + { + update_if_table(); + entry = find_entry_from_luid( (const NET_LUID *)key ); + } if (entry) { switch (param_type) @@ -591,9 +595,11 @@ static NTSTATUS index_luid_get_parameter( const void *key, UINT key_size, UINT p pthread_mutex_lock( &if_list_lock ); - update_if_table(); - - entry = find_entry_from_index( *(UINT *)key ); + if (!(entry = find_entry_from_index( *(UINT *)key ))) + { + update_if_table(); + entry = find_entry_from_index( *(UINT *)key ); + } if (entry) { *(NET_LUID *)data = entry->if_luid; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/78
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/nsiproxy.sys/ndis.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/nsiproxy.sys/ndis.c b/dlls/nsiproxy.sys/ndis.c index a3a10148109..929bda9cd90 100644 --- a/dlls/nsiproxy.sys/ndis.c +++ b/dlls/nsiproxy.sys/ndis.c @@ -644,7 +644,7 @@ BOOL convert_luid_to_unix_name( const NET_LUID *luid, const char **unix_name ) { *unix_name = entry->if_unix_name; ret = TRUE; - + break; } pthread_mutex_unlock( &if_list_lock ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/78
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/nsiproxy.sys/ndis.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/dlls/nsiproxy.sys/ndis.c b/dlls/nsiproxy.sys/ndis.c index 929bda9cd90..9b09756f35c 100644 --- a/dlls/nsiproxy.sys/ndis.c +++ b/dlls/nsiproxy.sys/ndis.c @@ -286,17 +286,19 @@ static struct if_entry *add_entry( UINT index, char *name ) return entry; } -static void update_if_table( void ) +static unsigned int update_if_table( void ) { struct if_nameindex *indices = if_nameindex(), *entry; + unsigned int append_count = 0; for (entry = indices; entry->if_index; entry++) { - if (!find_entry_from_index( entry->if_index )) - add_entry( entry->if_index, entry->if_name ); + if (!find_entry_from_index( entry->if_index ) && add_entry( entry->if_index, entry->if_name )) + ++append_count; } if_freenameindex( indices ); + return append_count; } static void if_counted_string_init( IF_COUNTED_STRING *str, const WCHAR *value ) @@ -634,18 +636,24 @@ BOOL convert_luid_to_unix_name( const NET_LUID *luid, const char **unix_name ) { struct if_entry *entry; BOOL ret = FALSE; + int updated = 0; pthread_mutex_lock( &if_list_lock ); - update_if_table(); - - LIST_FOR_EACH_ENTRY( entry, &if_list, struct if_entry, entry ) - if (entry->if_luid.Value == luid->Value) + do + { + LIST_FOR_EACH_ENTRY( entry, &if_list, struct if_entry, entry ) { - *unix_name = entry->if_unix_name; - ret = TRUE; - break; + if (entry->if_luid.Value == luid->Value) + { + *unix_name = entry->if_unix_name; + ret = TRUE; + goto done; + } } + } while (!updated++ && update_if_table()); + +done: pthread_mutex_unlock( &if_list_lock ); return ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/78
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/nsiproxy.sys/ndis.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/dlls/nsiproxy.sys/ndis.c b/dlls/nsiproxy.sys/ndis.c index 9b09756f35c..667d399b0cb 100644 --- a/dlls/nsiproxy.sys/ndis.c +++ b/dlls/nsiproxy.sys/ndis.c @@ -615,18 +615,24 @@ BOOL convert_unix_name_to_luid( const char *unix_name, NET_LUID *luid ) { struct if_entry *entry; BOOL ret = FALSE; + int updated = 0; pthread_mutex_lock( &if_list_lock ); - update_if_table(); - - LIST_FOR_EACH_ENTRY( entry, &if_list, struct if_entry, entry ) - if (!strcmp( entry->if_unix_name, unix_name )) + do + { + LIST_FOR_EACH_ENTRY( entry, &if_list, struct if_entry, entry ) { - *luid = entry->if_luid; - ret = TRUE; - break; + if (!strcmp( entry->if_unix_name, unix_name )) + { + *luid = entry->if_luid; + ret = TRUE; + goto done; + } } + } while (!updated++ && update_if_table()); + +done: pthread_mutex_unlock( &if_list_lock ); return ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/78
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/nsi/nsi.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/dlls/nsi/nsi.c b/dlls/nsi/nsi.c index efb10c0f615..345dce6dfe6 100644 --- a/dlls/nsi/nsi.c +++ b/dlls/nsi/nsi.c @@ -39,7 +39,7 @@ DWORD WINAPI NsiAllocateAndGetTable( DWORD unk, const NPI_MODULEID *module, DWOR void **rw_data, DWORD rw_size, void **dynamic_data, DWORD dynamic_size, void **static_data, DWORD static_size, DWORD *count, DWORD unk2 ) { - DWORD err, num = 0; + DWORD err, num = 64; void *data[4] = { NULL }; DWORD sizes[4] = { key_size, rw_size, dynamic_size, static_size }; int i, attempt; @@ -49,9 +49,6 @@ DWORD WINAPI NsiAllocateAndGetTable( DWORD unk, const NPI_MODULEID *module, DWOR for (attempt = 0; attempt < 5; attempt++) { - err = NsiEnumerateObjectsAllParameters( unk, 0, module, table, NULL, 0, NULL, 0, NULL, 0, NULL, 0, &num ); - if (err) return err; - for (i = 0; i < ARRAY_SIZE(data); i++) { if (sizes[i]) @@ -68,9 +65,11 @@ DWORD WINAPI NsiAllocateAndGetTable( DWORD unk, const NPI_MODULEID *module, DWOR err = NsiEnumerateObjectsAllParameters( unk, 0, module, table, data[0], sizes[0], data[1], sizes[1], data[2], sizes[2], data[3], sizes[3], &num ); if (err != ERROR_MORE_DATA) break; - + TRACE( "Short buffer, attempt %d.\n", attempt ); NsiFreeTable( data[0], data[1], data[2], data[3] ); memset( data, 0, sizeof(data) ); + err = NsiEnumerateObjectsAllParameters( unk, 0, module, table, NULL, 0, NULL, 0, NULL, 0, NULL, 0, &num ); + if (err) return err; } if (!err) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/78
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/iphlpapi/iphlpapi_main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c index 0bd7cdc06c8..5a4acdb9c5a 100644 --- a/dlls/iphlpapi/iphlpapi_main.c +++ b/dlls/iphlpapi/iphlpapi_main.c @@ -598,7 +598,6 @@ DWORD WINAPI GetAdaptersInfo( IP_ADAPTER_INFO *info, ULONG *size ) DWORD len, i, uni, fwd; NET_LUID *if_keys = NULL; struct nsi_ndis_ifinfo_rw *if_rw = NULL; - struct nsi_ndis_ifinfo_dynamic *if_dyn = NULL; struct nsi_ndis_ifinfo_static *if_stat = NULL; struct nsi_ipv4_unicast_key *uni_keys = NULL; struct nsi_ip_unicast_rw *uni_rw = NULL; @@ -612,7 +611,8 @@ DWORD WINAPI GetAdaptersInfo( IP_ADAPTER_INFO *info, ULONG *size ) err = NsiAllocateAndGetTable( 1, &NPI_MS_NDIS_MODULEID, NSI_NDIS_IFINFO_TABLE, (void **)&if_keys, sizeof(*if_keys), (void **)&if_rw, sizeof(*if_rw), - (void **)&if_dyn, sizeof(*if_dyn), (void **)&if_stat, sizeof(*if_stat), &if_count, 0 ); + NULL, 0, (void **)&if_stat, sizeof(*if_stat), &if_count, 0 ); + if (err) return err; for (i = 0; i < if_count; i++) { @@ -726,7 +726,7 @@ err: heap_free( wins_servers ); NsiFreeTable( fwd_keys, NULL, NULL, NULL ); NsiFreeTable( uni_keys, uni_rw, NULL, NULL ); - NsiFreeTable( if_keys, if_rw, if_dyn, if_stat ); + NsiFreeTable( if_keys, if_rw, NULL, if_stat ); return err; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/78
v3: - avoid redundant update in patches 3, 4 (sorry, that was an overlook); - do {} while formatting. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/78#note_947
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/78
participants (3)
-
Huw Davies (＠huw) -
Paul Gofman
-
Paul Gofman (＠gofman)