Per the following RFC: https://github.com/devtools-html/rfcs/issues/48 And in order to unblock Promise.jsm removal and especially allow landing bug 1388054, we should refactor inspector.js to be loaded as a module, via DevTools module loader.

Unfortunately, refactoring the inspector isn't that simple. It looks like loading the modules from a dedicated browser loader introduce a significant performance regression: https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=mozilla-central&newProject=try&newRevision=3bf308f1562c6115c7c474e14037138f5b20123b&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyImportant=1&framework=1&selectedTimeRange=172800 Replacing the toolbox.browserRequire() calls by browser-loader's require and using commonLibRequire doesn't change anything: https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=mozilla-central&newProject=try&newRevision=a91e37e1d30ee74495d7b4c5aac24a9660690a12&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyImportant=1&framework=1&selectedTimeRange=172800 (I was not expecting any performance change here, that should just be another way to write the same thing) It isn't really clear why it is any slower. The only thing that comes to mind would be that we preload many modules from devtools/client/inspector from the toolbox or other initialization code. By preload I mean that they are loaded in the DevTools loader before the panel opens. And now, with the browser loader, we have to load them again. Otherwise, it would mean that BrowserLoader is significantly slower than the Devtools loader?!?

So, I tried looking if this slowdown was related to cross compartment wrappers that appears when you switch from a page script to a module. For now, all modules are loaded into a privileged sandbox and so are running in another global and compartment, different from the page. So all references from modules to page objects (DOM Element, JS objects, ... everything that comes from panelWin) are cross compartment wrappers when used from modules. Here is a first try run, showing the effect of a patch that allows running browser loader modules in the same compartment than the panel they are attached to: https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=b2fe7b5c55b7496223bc8dd8d3be35d371354c8c&newProject=try&newRevision=85fbd30abb06e7e807155603de4bc30d8a90e47a&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyImportant=1&framework=1 This patch is without this inspector patch. It reports mostly speedups in debugger, netmonitor and console, mostly on reload and a couple of panel-specific operations. It also affects panelsInBackground. But it doesn't change anything to inspector perf. It seems to regress debugger and netmonitor opening a bit. Now, the same patch applied on top of the second patch of this queue, attachment 8988485 [details]: https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=9679dff305f530490a2d56728054280e392969dc&newProject=try&newRevision=c1c7f1e038513f82636fbf055127dabc17b43043&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyImportant=1&framework=1 We get similar results except that we now see an impact on the inspector: * 3.5% regression on custom.open * 11% improvement on custom.inspector.manyrules.selectnode Another view of the same patch, but comparing against m-c: https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=b2fe7b5c55b7&newProject=try&newRevision=c1c7f1e038513f82636fbf055127dabc17b43043&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyImportant=1&framework=1 It says that this browser loader patch doesn't solve the performance regression of panel initialization refactoring :/ Otherwise, in my last comment I gave PerfHerder comparison pages without a custom base revision, here is one with a custom base comparing with the second patch of this queue: https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=b2fe7b5c55b7496223bc8dd8d3be35d371354c8c&newProject=try&newRevision=9679dff305f530490a2d56728054280e392969dc&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyImportant=1&framework=1

I'm now wondering if that could be due to wrappers between the two loaders. With and without this patch to change the compartment of browser loader, both main loader and browser loaders are being ran in different compartment, also leading to cross compartment wrappers. But it doesn't make sense here, as the initialization patch should change much the story about wrappers. We were already having wrappers between page scripts and everything else (browser loaders/main loader modules). I'll look into that next week.

I started looking at the profile generated on try: * Without the patch: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FWuzKEaLtTGaAONk3pZDJ-A%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_damp.zip * With the patch: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FOF9hfR01Sa2XbLgPFGgCkA%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_damp.zip And it looks like, ElementEditor._createAttribute is getting slower to execute as it cross a compartments when calling parseAttribute, while it wasn't before. https://searchfox.org/mozilla-central/rev/97d488a17a848ce3bebbfc83dc916cf20b88451c/devtools/client/inspector/markup/views/element-editor.js#355 https://searchfox.org/mozilla-central/rev/97d488a17a848ce3bebbfc83dc916cf20b88451c/devtools/client/shared/node-attribute-parser.js#190 When looking at the profile, it is obvious that we go through wrappers for this precise call to parseAttribute from _createAttribute, but I'm having a hard time proving that it is actually slower... devtools/client/inspector/markup/views/element-editor.js is now being loaded via the inspector's browser loader whereas devtools/client/shared/node-attribute-parser.js is loaded via the main devtools loader. Before this patch, both modules were loaded in the main devtools loader. devtools/client/inspector/inspector.js was loading markup.js via the main loader: https://searchfox.org/mozilla-central/source/devtools/client/inspector/inspector.js#30 and that's the one module that ends up loading element-editor via intermediate dependencies. I'll try to use the inspector browser loader only for inspector.js and see if that changes anything to DAMP results.

(In reply to Alexandre Poirot [:ochameau] from comment #8) > I'll try to use the inspector browser loader only for inspector.js and see > if that changes anything to DAMP results. When I do that, there is no perf regression: https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=2ecbbe8b61fe135d7e05c09bdb8e78761c41863a&newProject=try&newRevision=b614d0bc89b6cb42a39ce75864a78d1462fa52f6&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&framework=1 So. I think we only made inspector.js change from script tag to module loader. We can conclude that it isn't an issue between loading something in script tag, versus, loading something in the loader. So that this regression doesn't highlight bug 1430656 and the lack of JS optimization when files are loaded via the loader. I tend to think now that it is all about compartments and wrappers. And more precisely, wrappers between browser loader modules and devtools loader modules. And not the wrappers between modules and page objects as they were already there before these patches as all but inspector.js were already loaded as modules.

I can confirm that the regressions to selectnode/collapse are related to the wrappers between browser loader modules and main loader modules. If I first apply a patch to make it so that both loaders share the same compartment and are no longer having wrapper between them, we only see regressions on open: https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=d42c445c151d1173bac454c57689bdff41fbfddd&newProject=try&newRevision=dc6f885b8ff4286b5b8865ef6c255fe3979afa02&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyImportant=1&framework=1 Regressions on open may be related to modules that we load twice. Modules that used to already be loaded in the main loader and that we load a second time in the browser loader. It can be anything from devtools/client/inspector/.