While working on figuring out why bug 1449162 regress performances, I realized that Promise.jsm stacks were very visible in perf-html profile. And it looks like replacing Promise.jsm usages in favor of DOM Promises improves the tests that use to regress when converting netmonitor actors that I tried to convert to procotol.js. https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=05b1f4ebfd22&newProject=try&newRevision=751b03561072ed409796b58b735825d76e5baf05&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyConfident=1&framework=1

It looks like we can stop using Promise.jsm from defer module without errors: https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa369d2480020ba2b2fe0d3afbe4e2aa2c78e6a7

Comment on attachment 8968188 [details] Bug 1454373 - Switch protocol.js to native promises. https://reviewboard.mozilla.org/r/236866/#review242712 Great, thanks for doing this! :)

Comment on attachment 8968230 [details] Bug 1454373 - Make defer module use DOM Promises. https://reviewboard.mozilla.org/r/236922/#review242714 ::: devtools/shared/defer.js (Diff revision 1) > * License, v. 2.0. If a copy of the MPL was not distributed with this > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > "use strict"; > > -// See bug 1273941 to understand this choice of promise. Hmm, the bug here doesn't really help me understand why this was put here, haha... :S Anyway, we do want to remove it, so hopefully it works.

Comment on attachment 8968230 [details] Bug 1454373 - Make defer module use DOM Promises. https://reviewboard.mozilla.org/r/236922/#review242714 > Hmm, the bug here doesn't really help me understand why this was put here, haha... :S > > Anyway, we do want to remove it, so hopefully it works. Ah, I guess it was about rejection tracking, but hopefully that's fixed now.

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #7) > Comment on attachment 8968230 [details] > Bug 1454373 - Make defer module use DOM Promises. > > https://reviewboard.mozilla.org/r/236922/#review242714 > > > Hmm, the bug here doesn't really help me understand why this was put here, haha... :S > > > > Anyway, we do want to remove it, so hopefully it works. > > Ah, I guess it was about rejection tracking, but hopefully that's fixed now. Yes, that was it. I hope it is properly handled now. If not, we have to fix it ASAP as a lot of code now use DOM Promise. While I was working on Promise.jsm, I used to miss some frames in stacks, but it seems to no longer be true. I think it is thanks to bug 1438121. Before, this protcol.js test I'm modifying here, used to miss the "onConnect" frame completely!

DAMP still looks very green with defer switch to DOM Promises: https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=05b1f4ebfd2248b3a29195639528f20a560f87a4&newProject=try&newRevision=bdbbda668634f6a4c52d355a78a6352557ad293f&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyConfident=1&framework=1 It looks like it brings a small win in many tests.

Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b2ada4fddfa0 Switch protocol.js to native promises. r=jryans https://hg.mozilla.org/integration/autoland/rev/4ec712f0aa13 Make defer module use DOM Promises. r=jryans

Backed out for mochitest devtools failures on "OS X 10.10 opt" - devtools/client/shadereditor/test/browser_se_* Backout link: https://hg.mozilla.org/integration/autoland/rev/3ee4cf8dc2ed4ae1b81d229398706e243f3c7c3f Log: https://treeherder.mozilla.org/logviewer.html#?job_id=174098374&repo=autoland

Hum. I knew it wouldn't be so easy... I don't get why this test passed in this try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa369d2480020ba2b2fe0d3afbe4e2aa2c78e6a7 While the test is failing locally, on my linux...

So it is about Promise.jsm again, and the famous DOM Promise that are zombified on panel's iframe removal. *But* this time we are not involving any panel document's iframe! So it doesn't really make sense... We already hit the exact same code in bug 1444755. Some destruction code within Shader Editor starts failing. A promise never resolve. The promise is this one: https://searchfox.org/mozilla-central/rev/a30a60eadcff99d2a043241955d215dbeccad876/devtools/client/shadereditor/shadereditor.js#439 deferred.resolve(editor); It starts failing i.e. it no longer resolves, i.e. the callsite's `then` function argument is no longer called: https://searchfox.org/mozilla-central/rev/a30a60eadcff99d2a043241955d215dbeccad876/devtools/client/shadereditor/shadereditor.js#457 return this._getEditor(type).then(editor => { The arrow function here, is never called. It starts failing once we remove the use of Promise.jsm in defer module: https://searchfox.org/mozilla-central/rev/a30a60eadcff99d2a043241955d215dbeccad876/devtools/shared/defer.js#8 - const Promise = require("promise"); The following change still fails, even if `Promise` symbol is a special DOM Promise, coming from module loader Sandbox and supposed to be always alive and working! https://searchfox.org/mozilla-central/rev/a30a60eadcff99d2a043241955d215dbeccad876/devtools/client/shadereditor/shadereditor.js#439 - deferred.resolve(editor); + return Promise.resolve(editor); It also still fails if I switch from Promise.jsm to these special Promises in defer: https://searchfox.org/mozilla-central/rev/a30a60eadcff99d2a043241955d215dbeccad876/devtools/shared/defer.js#8 - const Promise = require("promise"); + const Promise = require("Promise"); Note that we should not even need that. defer.js is loaded in a non-browser-loader sandbox and should be always alive. The only code I found that was still working, was to do: https://searchfox.org/mozilla-central/rev/a30a60eadcff99d2a043241955d215dbeccad876/devtools/client/shadereditor/shadereditor.js#439 - deferred.resolve(editor); + return { + then(a) { + a(editor); + } + }; But this is far from being satisfying!

So. Modifying defer.js ends up being a much bigger project, related to Promise.jsm removal from client usages. It would require to stop involving any promise for all panels during destruction. Which is a very large project as we would also have to do that for unmaintained panels. What I don't understand is why require("Promise") doesn't work. Why even these promises get frozen and never resolves. That would be the only workaround I know about to not have to refactor everything. In the meantime, I would like to suggest embedding a copy of defer into protocol.js that doesn't use Promise.jsm...

Comment on attachment 8968188 [details] Bug 1454373 - Switch protocol.js to native promises. Could you take another look and see if you agree on the defer workaround?

Comment on attachment 8968188 [details] Bug 1454373 - Switch protocol.js to native promises. https://reviewboard.mozilla.org/r/236866/#review243180 Sorry to hear about the pain you went through! :( A workaround like this seems fine to me given the perf improvement. Could you also add a comment in defer.js to point future attempts to change it towards your experience?

Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cde33317ae7a Switch protocol.js to native promises. r=jryans