Closed
Bug 461566
Opened 16 years ago
Closed 16 years ago
Don't call FindTearoff when not needed and cache XPCNativeInterfaces in quickstubs
Categories
(Core :: XPConnect, defect, P1)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla1.9.1b3
People
(Reporter: peterv, Assigned: peterv)
References
Details
(Keywords: fixed1.9.1)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
A lot of callers of WrapNative know that the object they have implements the interface for which they want to wrap. We can avoid creating XPCWrappedNativeTearOff in those cases, but we must be careful not to do this for tearoffs to make sure those are kept alive by the wrapper.
Assignee | ||
Comment 1•16 years ago
|
||
Slightly changing the scope of this bug. Turns out it's hard to know when we can skip calling FindTearoff. The one case that seems certain is when wrapping for nsISupports. We could allow callers to pass in null for the IID, which would mean wrap for nsISupports, and then skip FindTearoff. The biggest cost in FindTearoff seems to be looking up the XPCNativeInterface (hash lookup with locking). Quickstubs have a known set of result types, so we can cache the XPCNativeInterfaces for all result types and avoid that cost most of the time. Patch coming up that does both these things. FindTearoff is still showing up in profiles, but for now I don't think there's an easy way to avoid it.
Priority: -- → P2
Summary: Don't create XPCWrappedNativeTearOff when not needed → Don't call FindTearoff when not needed and cache XPCNativeInterfaces in quickstubs
Target Milestone: --- → mozilla1.9.1
Assignee | ||
Comment 2•16 years ago
|
||
Attachment #344659 -
Attachment is obsolete: true
Attachment #349176 -
Flags: superreview?(jst)
Attachment #349176 -
Flags: review?(jst)
Updated•16 years ago
|
Attachment #349176 -
Flags: superreview?(jst)
Attachment #349176 -
Flags: superreview+
Attachment #349176 -
Flags: review?(jst)
Attachment #349176 -
Flags: review+
Comment 3•16 years ago
|
||
Comment on attachment 349176 [details] [diff] [review] v2 Looks good! I like the fact that the interface table in dom_quickstubs.cpp only needs to contain interfaces for types that are ever returned, which means the list is surprisingly small. r+sr=jst
Assignee | ||
Comment 4•16 years ago
|
||
We really want these performance improvements in 1.9.1.
Flags: blocking1.9.2?
Assignee | ||
Updated•16 years ago
|
Flags: blocking1.9.2? → blocking1.9.1?
Comment 5•16 years ago
|
||
This is a significant performance gain in our JS -> C++ path, and I think we should take this before shipping. Blocking, and bumping to P1 as having this in a beta would be better than not.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: P2 → P1
Comment 6•16 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/037f635ced9f
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.9.1 → mozilla1.9.1b3
Comment 7•16 years ago
|
||
I had to back this out since it did not compile on windows: nsScriptError.cpp Building deps for /e/builds/buildbot/comm-central-win32/build/mozilla/js/src/xpconnect/src/nsScriptError.cpp cl -FonsScriptError.obj -c -DXPCOM_TRANSLATE_NSGM_ENTRY_POINT=1 -DMOZILLA_INTERNAL_API -D_IMPL_NS_GFX -D_IMPL_NS_MSG_BASE -D_IMPL_NS_WIDGET -DMOZ_THUNDERBIRD=1 -DOSTYPE=\"WINNT5.2\" -DOSARCH=WINNT -DJSFILE -DJS_THREADSAFE -DEXPORT_XPC_API -DXPC_IDISPATCH_SUPPORT -I/e/builds/buildbot/comm-central-win32/build/mozilla/js/src/xpconnect/src/../loader -I/e/builds/buildbot/comm-central-win32/build/mozilla/js/src/xpconnect/src -I. -I../../../../dist/include/xpcom -I../../../../dist/include/string -I../../../../dist/include/js -I../../../../dist/include/caps -I../../../../dist/include/necko -I../../../../dist/include/dom -I../../../../dist/include/content -I../../../../dist/include/editor -I../../../../dist/include/layout -I../../../../dist/include/rdf -I../../../../dist/include/svg -I../../../../dist/include/xuldoc -I../../../../dist/include/xultmpl -I../../../../dist/include -I../../../../dist/include/xpconnect -Ie:/builds/buildbot/comm-central-win32/build/objdir-tb/mozilla/dist/include/nspr -Ie:/builds/buildbot/comm-central-win32/build/objdir-tb/mozilla/dist/sdk/include -GR- -TP -nologo -Zc:wchar_t- -W3 -Gy -Fdgenerated.pdb -DNDEBUG -DTRIMMED -Zi -UDEBUG -DNDEBUG -O1 -MD -D_CRT_SECURE_NO_DEPRECATE=1 -D_CRT_NONSTDC_NO_DEPRECATE=1 -DWINVER=0x500 -D_WIN32_WINNT=0x500 -D_WIN32_IE=0x0500 -DX_DISPLAY_MISSING=1 -DMOZILLA_VERSION=\"1.9.1b3pre\" -DMOZILLA_VERSION_U=1.9.1b3pre -DHAVE_SNPRINTF=1 -D_WINDOWS=1 -D_WIN32=1 -DWIN32=1 -DXP_WIN=1 -DXP_WIN32=1 -DHW_THREADS=1 -DSTDC_HEADERS=1 -DWIN32_LEAN_AND_MEAN=1 -DNO_X11=1 -DHAVE_MMINTRIN_H=1 -DHAVE_OLEACC_IDL=1 -DHAVE_ATLBASE_H=1 -DHAVE_WPCAPI_H=1 -D_X86_=1 -DD_INO=d_ino -DMOZ_EMBEDDING_LEVEL_DEFAULT=1 -DMOZ_EMBEDDING_LEVEL_BASIC=1 -DMOZ_EMBEDDING_LEVEL_MINIMAL=1 -DMOZ_BUILD_APP=../mail -DMOZ_XUL_APP=1 -DMOZ_DEFAULT_TOOLKIT=\"cairo-windows\" -DMOZ_DISTRIBUTION_ID=\"org.mozilla\" -DIBMBIDI=1 -DMOZ_VIEW_SOURCE=1 -DACCESSIBILITY=1 -DMOZ_XPINSTALL=1 -DMOZ_JSLOADER=1 -DNS_PRINTING=1 -DNS_PRINT_PREVIEW=1 -DMOZ_NO_XPCOM_OBSOLETE=1 -DMOZ_OGG=1 -DMOZ_WAVE=1 -DMOZ_SYDNEYAUDIO=1 -DMOZ_MEDIA=1 -DMOZ_XTF=1 -DMOZ_CRASHREPORTER=1 -DMOZ_CRASHREPORTER_ENABLE_PERCENT=100 -DMOZ_ENABLE_CANVAS=1 -DMOZ_SVG=1 -DMOZ_UPDATE_CHANNEL=nightly -DMOZ_FEEDS=1 -DMOZ_STORAGE=1 -DMOZ_SAFE_BROWSING=1 -DMOZ_URL_CLASSIFIER=1 -DMOZ_LOGGING=1 -DMOZ_USER_DIR=\"Mozilla\" -DMOZ_STATIC_BUILD=1 -DMOZ_TREE_CAIRO=1 -DHAVE_UINT64_T=1 -DMOZ_XUL=1 -DMOZ_PROFILELOCKING=1 -DMOZ_RDF=1 -DMOZ_MORK=1 -DMOZ_DLL_SUFFIX=\".dll\" -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIENT /e/builds/buildbot/comm-central-win32/build/mozilla/js/src/xpconnect/src/nsScriptError.cpp nsScriptError.cpp e:\builds\buildbot\comm-central-win32\build\mozilla\js\src\xpconnect\src\xpcprivate.h(2768) : error C2332: 'struct' : missing tag name e:\builds\buildbot\comm-central-win32\build\mozilla\js\src\xpconnect\src\xpcprivate.h(2768) : error C2011: '<unnamed-tag>' : 'enum' type redefinition D:\sdks\v6.0\include\shlwapi.h(1550) : see declaration of '<unnamed-tag>' e:\builds\buildbot\comm-central-win32\build\mozilla\js\src\xpconnect\src\xpcprivate.h(2768) : error C2144: syntax error : '<unnamed-tag>' should be preceded by ')' e:\builds\buildbot\comm-central-win32\build\mozilla\js\src\xpconnect\src\xpcprivate.h(2768) : error C2144: syntax error : '<unnamed-tag>' should be preceded by ';' e:\builds\buildbot\comm-central-win32\build\mozilla\js\src\xpconnect\src\xpcprivate.h(2768) : error C2059: syntax error : ',' e:\builds\buildbot\comm-central-win32\build\mozilla\js\src\xpconnect\src\xpcprivate.h(2772) : error C2059: syntax error : ')' e:\builds\buildbot\comm-central-win32\build\mozilla\js\src\xpconnect\src\xpcprivate.h(2772) : error C2238: unexpected token(s) preceding ';' e:\builds\buildbot\comm-central-win32\build\mozilla\js\src\xpconnect\src\xpcprivate.h(2785) : error C2661: 'XPCConvert::NativeInterface2JSObject' : no overloaded function takes 10 arguments http://hg.mozilla.org/mozilla-central/rev/1b45760df6ee
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•16 years ago
|
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Keywords: fixed1.9.1
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•