The add-ons team plans to remove large portions of SDK code after 57. I'm assuming we'll keep using the SDK loader for the moment, but let's work out which SDK modules we need to drop. Here's a usage count of SDK modules in /devtools: 6 "sdk/clipboard" 1 "sdk/console/plain-text" 2 "sdk/content/mod" 50 "sdk/core/heritage" 1 "sdk/dom/events" 77 "sdk/event/core" 15 "sdk/event/target" 2 "sdk/indexed-db" 2 "sdk/io/buffer" 3 "sdk/lang/functional" 1 "sdk/loader/sandbox" 1 "sdk/preferences/service" 2 "sdk/stylesheet/style" 3 "sdk/stylesheet/utils" 2 "sdk/system/child_process/subprocess" 1 "sdk/system/environment" 2 "sdk/system/events" 1 "sdk/system/runtime" 1 "sdk/system/unload" 2 "sdk/tabs" 5 "sdk/tabs/utils" 4 "sdk/timers" 2 "sdk/url" 15 "sdk/util/object" 1 "sdk/util/uuid" 1 "sdk/view/core" 2 "sdk/window/helpers" 8 "sdk/window/utils"

:kmag, I've read in various places that the add-ons team intends to remove some of SDK code after 57, but I haven't heard a precise list. Will all of the things above be removed, or only some?

See bug 1350646. Of the above, the modules that I want to remove as soon as possible are: "sdk/content/mod" "sdk/dom/events" "sdk/loader/sandbox" "sdk/system/child_process/subprocess" "sdk/system/events" "sdk/system/unload" "sdk/tabs" "sdk/tabs/utils" "sdk/view/core" "sdk/window/helpers" And the following sooner rather than later: "sdk/url" "sdk/util/object" "sdk/indexed-db" "sdk/preferences/service" "sdk/io/buffer" The sdk/core/heritage stuff is not great for perf, so it would be great if you could migrate to ES6 classes, but I'm not in a hurry to remove it.

sdk/system/child_process/subprocess is used in WebIDE, which bug 1314811 aims to remove.

(In reply to Kris Maglione [:kmag] from comment #2) > The sdk/core/heritage stuff is not great for perf, so it would be great if > you could migrate to ES6 classes, but I'm not in a hurry to remove it. I would love to migrate to ES6 classes, however it won't be great for perf too, we need bug 1167472 fixed for that. (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #0) > 77 "sdk/event/core" > 15 "sdk/event/target" I'd like to keep those, maybe we could update them and move in devtools codebase; I found them better to use in some scenario than the `EventEmitter` we have in devtools – we could also think about refactoring `EventEmitter` too. I could work on them since I was part of the original Add-on SDK team, if it's needed.

(In reply to Matteo Ferretti [:zer0] [:matteo] from comment #4) > (In reply to Kris Maglione [:kmag] from comment #2) > > The sdk/core/heritage stuff is not great for perf, so it would be great if > > you could migrate to ES6 classes, but I'm not in a hurry to remove it. > > I would love to migrate to ES6 classes, however it won't be great for perf > too, we need bug 1167472 fixed for that. I'd be willing to bet that whatever the perf cost, it's less than the cost of core/heritage. As I understand it, classes aren't JITtable under certain circumstances, mainly in the scope where they're declared, but not when they're used. If you're planning on making the switch in the future, anyway, it might be worth migrating some classes now and testing the perf impact, and just waiting to land if it causes trouble. > (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #0) > > 77 "sdk/event/core" > > 15 "sdk/event/target" > > I'd like to keep those, maybe we could update them and move in devtools > codebase; I found them better to use in some scenario than the > `EventEmitter` we have in devtools – we could also think about refactoring > `EventEmitter` too. > > I could work on them since I was part of the original Add-on SDK team, if > it's needed. Well, I don't have any say over what you do in the devtools module, so you're free to move those modules there if you want. I'd advise against it, though. From the profiling I've done, the event modules add a lot of overhead in a lot of places. It may not be as much of an issue for the devtools uses as it is for some of the extreme ways the SDK uses it, and it's gotten better after some recent changes, but I still think you'd be better off with the simpler EventTarget class used elsewhere, that doesn't rely so heavily on WeakMaps and the `method` gunk used by the SDK.

> (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #0) > > > 77 "sdk/event/core" > > 15 "sdk/event/target" > > I'd like to keep those, maybe we could update them and move in devtools > codebase; I found them better to use in some scenario than the > `EventEmitter` we have in devtools – we could also think about refactoring > `EventEmitter` too. We do have bug 952653 to hopefully someday use only one event system in DevTools. `event-emitter` seems more appealing to me these days, since it seems to have better perf than sdk/event/core, and it's also used in various Firefox modules as well. Maybe we can explore using bug 952653 for the sdk/event portion here.

In total I'm currently seeing 178 occurrences of requiring the sdk. http://searchfox.org/mozilla-central/search?q=%5C(%5B%22%27%5Dsdk%2F&case=false&regexp=true&path=devtools%2F

Down to 159 occurrences, progress but not quite moving fast enough.

Don't forget not to miss the modules with simplified mappings e.g.: `require("timer")` can be used instead of `require("sdk/timers")`. Here is the full list of simplified mappings from addon-sdk/source/mapping.json: { "api-utils": "sdk/deprecated/api-utils", "base64": "sdk/base64", "content": "sdk/content/content", "deprecate": "sdk/util/deprecate", "event/core": "sdk/event/core", "events": "sdk/deprecated/events", "functional": "sdk/core/functional", "l10n/core": "sdk/l10n/json/core", "l10n/html": "sdk/l10n/html", "l10n/loader": "sdk/l10n/loader", "l10n/locale": "sdk/l10n/locale", "l10n/prefs": "sdk/l10n/prefs", "list": "sdk/util/list", "loader": "sdk/loader/loader", "namespace": "sdk/core/namespace", "preferences-service": "sdk/preferences/service", "promise": "sdk/core/promise", "system": "sdk/system", "system/events": "sdk/system/events-shimmed", "tabs/tab": "sdk/tabs/tab", "tabs/utils": "sdk/tabs/utils", "timer": "sdk/timers", "traits": "sdk/deprecated/traits", "unload": "sdk/system/unload", "window-utils": "sdk/deprecated/window-utils", "window/utils": "sdk/window/utils", "windows/dom": "sdk/windows/dom", "windows/loader": "sdk/windows/loader", "xul-app": "sdk/system/xul-app", "url": "sdk/url", "traceback": "sdk/console/traceback", "xhr": "sdk/net/xhr", "match-pattern": "sdk/util/match-pattern", "file": "sdk/io/file", "runtime": "sdk/system/runtime", "xpcom": "sdk/platform/xpcom", "querystring": "sdk/querystring", "text-streams": "sdk/io/text-streams", "app-strings": "sdk/deprecated/app-strings", "environment": "sdk/system/environment", "keyboard/utils": "sdk/keyboard/utils", "dom/events": "sdk/dom/events-shimmed", "utils/data": "sdk/io/data", "test/assert": "sdk/test/assert", "hidden-frame": "sdk/frame/hidden-frame", "collection": "sdk/util/collection", "array": "sdk/util/array", "clipboard": "sdk/clipboard", "context-menu": "sdk/context-menu", "hotkeys": "sdk/hotkeys", "indexed-db": "sdk/indexed-db", "l10n": "sdk/l10n", "notifications": "sdk/notifications", "page-mod": "sdk/page-mod", "page-worker": "sdk/page-worker", "panel": "sdk/panel", "passwords": "sdk/passwords", "private-browsing": "sdk/private-browsing", "request": "sdk/request", "selection": "sdk/selection", "self": "sdk/self", "simple-prefs": "sdk/simple-prefs", "simple-storage": "sdk/simple-storage", "tabs": "sdk/tabs", "timers": "sdk/timers", "windows": "sdk/windows", "harness": "sdk/test/harness", "run-tests": "sdk/test/runner", "test": "sdk/test" }

Quick update on current usage: $ egrep -Eoh "\"sdk/.*\"" -r devtools/ | sort | uniq -c 5 "sdk/clipboard" 15 "sdk/core/heritage" 61 "sdk/event/core" 6 "sdk/event/target" 2 "sdk/indexed-db" 1 "sdk/lang/functional/concurrent.js" 2 "sdk/system/child_process/subprocess" [should be removed soon in bug 1353656] 1 "sdk/system/environment" [should be removed shortly in bug 1378833] 1 "sdk/system/events" [only one test [1]] 1 "sdk/system/runtime" [should be removed shortly in bug 1378835] 4 "sdk/timers" 10 "sdk/util/object" 2 "sdk/util/uuid" :jryans, is this a command similar to the one you used in comment 0? From kris's comment 2, we still have some uses of the following: "sdk/system/child_process/subprocess" [should be removed soon in bug 1353656] "sdk/system/environment" [should be removed shortly in bug 1378833] "sdk/system/events" [only used in a test [1]] "sdk/system/runtime" [should be removed shortly in bug 1378835] "sdk/util/object" [Mostly used in performance panel and in protocol.js] "sdk/indexed-db" [I'm afraid this one would have to be copied] [1]: http://searchfox.org/mozilla-central/source/devtools/client/debugger/test/mochitest/addon-source/browser_dbg_addon3/lib/main.js#2

(In reply to Alexandre Poirot [:ochameau] from comment #10) > :jryans, is this a command similar to the one you used in comment 0? Yes, looks similar! :)

Weekly update: $ egrep -Eoh "\"sdk/.*\"" -r devtools/ | sort | uniq -c 4 "sdk/clipboard" (bug 1378820, landed but not on the git mirror yet?) 12 "sdk/core/heritage" (also blocked on bug 1381542) 61 "sdk/event/core" (bug 1381542) 6 "sdk/event/target" (bug 1381542) 2 "sdk/indexed-db" (bug 1361333, r+) 1 "sdk/lang/functional/concurrent.js" [1] 1 "sdk/system/events" [2] 4 "sdk/timers" [3] 10 "sdk/util/object" (bug 1361332, in autoland) So most of everything is soon to be blocked on bug 1381542, I'm introducing some delay by suggesting waiting for tomorrow to land the event-emitter refactoring on FF57. I also do an extensive review of the new API, which introduces some delay. [1] This is a false report from a comment: http://searchfox.org/mozilla-central/source/devtools/shared/debounce.js#13 [2] We may miss a bug for the last usage of sdk/system/events: http://searchfox.org/mozilla-central/source/devtools/client/debugger/test/mochitest/addon-source/browser_dbg_addon3/lib/main.js#2 [3] I haven't found a bug to get rid of sdk/timers?

Just opened bug 1386357 to track sdk/timers removal.

$ egrep -Eoh "\"sdk/.*\"" -r devtools/ | sort | uniq -c 2 "sdk/core/heritage" One being in comment, here, that we can safely ignore: http://searchfox.org/mozilla-central/source/devtools/shared/extend.js#11 But what about this one? Is that just a leftover of `Class` is still used somewhere? http://searchfox.org/mozilla-central/source/devtools/server/performance/recorder.js#11-12

(In reply to Alexandre Poirot [:ochameau] from comment #14) > But what about this one? Is that just a leftover of `Class` is still used > somewhere? > http://searchfox.org/mozilla-central/source/devtools/server/performance/ > recorder.js#11-12 I couldn't find any usage of this import. Seems safe to just remove it.

It is with great pleasure that I resolve this bug