From: Francis De Brabandere <francisdb@gmail.com> Identifier lookups that resolve into a named item's host dispatch called GetIDsOfNames on the host for every access, while native vbscript resolves a name once and reuses the DISPID afterwards, observable with a host that counts the queries. Cache resolved ids on the named item and drop the cache together with the item's dispatch. A 300k-iteration loop reading a host member now queries the host once instead of 300000 times, matching native. With a host that scans a 1600-entry member table per query the loop drops from 35 s to 0.1 s; with a trivial host the time is unchanged. --- dlls/vbscript/interp.c | 4 +- dlls/vbscript/tests/vbscript.c | 90 ++++++++++++++++++++++++++++++++++ dlls/vbscript/vbscript.c | 51 +++++++++++++++++++ dlls/vbscript/vbscript.h | 5 ++ 4 files changed, 148 insertions(+), 2 deletions(-) diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index 1b071d6f077..c92c3aff632 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -211,7 +211,7 @@ static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_ if(ctx->func->code_ctx->named_item && ctx->func->code_ctx->named_item->disp && !(ctx->func->code_ctx->named_item->flags & SCRIPTITEM_CODEONLY)) { - hres = disp_get_id(ctx->func->code_ctx->named_item->disp, name, invoke_type, TRUE, &id); + hres = named_item_get_disp_id(ctx->func->code_ctx->named_item, name, invoke_type, TRUE, &id); if(SUCCEEDED(hres)) { ref->type = REF_DISP; ref->u.d.disp = ctx->func->code_ctx->named_item->disp; @@ -242,7 +242,7 @@ static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_ LIST_FOR_EACH_ENTRY(item, &ctx->script->named_items, named_item_t, entry) { if((item->flags & SCRIPTITEM_GLOBALMEMBERS)) { - hres = disp_get_id(item->disp, name, invoke_type, FALSE, &id); + hres = named_item_get_disp_id(item, name, invoke_type, FALSE, &id); if(SUCCEEDED(hres)) { ref->type = REF_DISP; ref->u.d.disp = item->disp; diff --git a/dlls/vbscript/tests/vbscript.c b/dlls/vbscript/tests/vbscript.c index 6fe7893d3b0..9162435c7b8 100644 --- a/dlls/vbscript/tests/vbscript.c +++ b/dlls/vbscript/tests/vbscript.c @@ -304,15 +304,19 @@ static const IDispatchVtbl persistent_named_item_vtbl = { static IDispatch persistent_named_item = { &persistent_named_item_vtbl }; +static int shadow_method_dispid_queries, shadow_prop_dispid_queries; + static HRESULT WINAPI shadowing_GetIDsOfNames(IDispatch *iface, REFIID riid, LPOLESTR *names, UINT name_cnt, LCID lcid, DISPID *ids) { ok(name_cnt == 1, "name_cnt = %u\n", name_cnt); if(!wcscmp(names[0], L"shadowMethod")) { + shadow_method_dispid_queries++; *ids = 1; return S_OK; } if(!wcscmp(names[0], L"shadowProp")) { + shadow_prop_dispid_queries++; *ids = 2; return S_OK; } @@ -3080,6 +3084,91 @@ static void test_named_item_no_dim_routes_to_host(void) ok(!ref, "ref = %ld\n", ref); } +static void test_named_item_dispid_caching(void) +{ + IActiveScriptParse *parse; + IActiveScript *script; + HRESULT hres; + LONG ref; + + /* The engine resolves a host member name through GetIDsOfNames once and + * reuses the DISPID for subsequent lookups, within and across script + * chunks. */ + + script = create_vbscript(); + hres = IActiveScript_QueryInterface(script, &IID_IActiveScriptParse, (void**)&parse); + ok(hres == S_OK, "Could not get IActiveScriptParse: %08lx\n", hres); + + SET_EXPECT(GetLCID); + hres = IActiveScript_SetScriptSite(script, &ActiveScriptSite); + ok(hres == S_OK, "SetScriptSite failed: %08lx\n", hres); + CHECK_CALLED(GetLCID); + + hres = IActiveScript_AddNamedItem(script, L"shadowingItem", SCRIPTITEM_ISVISIBLE); + ok(hres == S_OK, "AddNamedItem failed: %08lx\n", hres); + + SET_EXPECT(OnStateChange_INITIALIZED); + hres = IActiveScriptParse_InitNew(parse); + ok(hres == S_OK, "InitNew failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_INITIALIZED); + + SET_EXPECT(OnStateChange_CONNECTED); + hres = IActiveScript_SetScriptState(script, SCRIPTSTATE_CONNECTED); + ok(hres == S_OK, "SetScriptState failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_CONNECTED); + + shadow_method_dispid_queries = 0; + shadow_prop_dispid_queries = 0; + + SET_EXPECT(OnEnterScript); + SET_EXPECT(OnLeaveScript); + SET_EXPECT(host_shadow_propput); + SET_EXPECT(host_shadow_propget); + SET_EXPECT(host_shadow_method); + hres = IActiveScriptParse_ParseScriptText(parse, L"shadowProp = 5\nx = shadowProp\nshadowMethod\n", + L"shadowingItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(OnLeaveScript); + CHECK_CALLED(host_shadow_propput); + CHECK_CALLED(host_shadow_propget); + CHECK_CALLED(host_shadow_method); + + ok(shadow_prop_dispid_queries == 1, "shadowProp resolved %d times, expected 1\n", shadow_prop_dispid_queries); + ok(shadow_method_dispid_queries == 1, "shadowMethod resolved %d times, expected 1\n", shadow_method_dispid_queries); + + /* a later script chunk reuses the cached ids too */ + SET_EXPECT(OnEnterScript); + SET_EXPECT(OnLeaveScript); + SET_EXPECT(host_shadow_propput); + SET_EXPECT(host_shadow_method); + hres = IActiveScriptParse_ParseScriptText(parse, L"shadowProp = 6\nshadowMethod\n", + L"shadowingItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(OnLeaveScript); + CHECK_CALLED(host_shadow_propput); + CHECK_CALLED(host_shadow_method); + + ok(shadow_prop_dispid_queries == 1, "shadowProp resolved %d times after second chunk, expected 1\n", + shadow_prop_dispid_queries); + ok(shadow_method_dispid_queries == 1, "shadowMethod resolved %d times after second chunk, expected 1\n", + shadow_method_dispid_queries); + + SET_EXPECT(OnStateChange_DISCONNECTED); + SET_EXPECT(OnStateChange_INITIALIZED); + SET_EXPECT(OnStateChange_CLOSED); + hres = IActiveScript_Close(script); + ok(hres == S_OK, "Close failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_DISCONNECTED); + CHECK_CALLED(OnStateChange_INITIALIZED); + CHECK_CALLED(OnStateChange_CLOSED); + + IActiveScriptParse_Release(parse); + ref = IActiveScript_Release(script); + ok(!ref, "ref = %ld\n", ref); +} + static void test_const_at_top_level(void) { IActiveScriptParse *parse; @@ -3521,6 +3610,7 @@ START_TEST(vbscript) test_named_item_sub_shadowing(); test_named_item_dim_shadowing(); test_named_item_no_dim_routes_to_host(); + test_named_item_dispid_caching(); test_const_at_top_level(); test_cross_parse_name_redef(); test_scriptdisp(); diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index ff28b3b17ef..23236f9a2e2 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -261,6 +261,54 @@ named_item_t *lookup_named_item(script_ctx_t *ctx, const WCHAR *name, unsigned f return NULL; } +typedef struct { + struct rb_entry entry; + DISPID id; + WCHAR name[1]; +} dispid_cache_entry_t; + +static int dispid_cache_cmp(const void *key, const struct rb_entry *entry) +{ + dispid_cache_entry_t *cache_entry = RB_ENTRY_VALUE(entry, dispid_cache_entry_t, entry); + return vbs_wcsicmp(key, cache_entry->name); +} + +static void free_dispid_cache_entry(struct rb_entry *entry, void *context) +{ + free(RB_ENTRY_VALUE(entry, dispid_cache_entry_t, entry)); +} + +static void clear_dispid_cache(named_item_t *item) +{ + rb_destroy(&item->dispid_cache, free_dispid_cache_entry, NULL); + rb_init(&item->dispid_cache, dispid_cache_cmp); +} + +HRESULT named_item_get_disp_id(named_item_t *item, BSTR name, vbdisp_invoke_type_t invoke_type, BOOL search_private, DISPID *id) +{ + dispid_cache_entry_t *cache_entry; + struct rb_entry *entry; + size_t name_size; + HRESULT hres; + + if((entry = rb_get(&item->dispid_cache, name))) { + *id = RB_ENTRY_VALUE(entry, dispid_cache_entry_t, entry)->id; + return S_OK; + } + + hres = disp_get_id(item->disp, name, invoke_type, search_private, id); + if(FAILED(hres)) + return hres; + + name_size = (lstrlenW(name) + 1) * sizeof(WCHAR); + if((cache_entry = malloc(offsetof(dispid_cache_entry_t, name[0]) + name_size))) { + memcpy(cache_entry->name, name, name_size); + cache_entry->id = *id; + rb_put(&item->dispid_cache, cache_entry->name, &cache_entry->entry); + } + return S_OK; +} + static void release_named_item_script_obj(named_item_t *item) { if(!item->script_obj) return; @@ -274,6 +322,7 @@ void release_named_item(named_item_t *item) { if(--item->ref) return; + clear_dispid_cache(item); free(item->name); free(item); } @@ -307,6 +356,7 @@ static void release_script(script_ctx_t *ctx) { IDispatch_Release(item->disp); item->disp = NULL; + clear_dispid_cache(item); } release_named_item_script_obj(item); if(!(item->flags & SCRIPTITEM_ISPERSISTENT)) @@ -869,6 +919,7 @@ static HRESULT WINAPI VBScript_AddNamedItem(IActiveScript *iface, LPCOLESTR pstr item->disp = disp; item->flags = dwFlags; item->script_obj = NULL; + rb_init(&item->dispid_cache, dispid_cache_cmp); item->name = wcsdup(pstrName); if(!item->name) { if(disp) diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index e3c57b0f8e5..cccfb47bfb3 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -164,9 +164,14 @@ typedef struct named_item_t { DWORD flags; LPWSTR name; + /* member names resolved against disp, valid while disp is */ + struct rb_tree dispid_cache; + struct list entry; } named_item_t; +HRESULT named_item_get_disp_id(named_item_t*,BSTR,vbdisp_invoke_type_t,BOOL,DISPID*); + HRESULT create_vbdisp(const class_desc_t*,vbdisp_t**); HRESULT disp_get_id(IDispatch*,BSTR,vbdisp_invoke_type_t,BOOL,DISPID*); const WCHAR *vbdisp_class_name(IDispatch*); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11136