Change communication protocol for DDC's dart:developer implementation

[grouma]

Currently DDC logs to the JS console whenever it needs to make a message available. This appears to have some performance overhead, especially if the Chrome inspector is open. A potential better alternative is to provide hooks on the page which DDC can detect and call directly.

cc @jakemac53
cc @vsmenon

[vsmenon]

Agreed. Do you have a sense of whether we need to do this now, or whether it can wait till later (post 2.3)?

[grouma]

This likely can wait post 2.3. I only notice performance issues when the Chrome Inspector is open. Surprisingly it is made even worse if you set the logging level to Default (which hides these type of messages). This implies that it is potentially a Chrome Inspector issue. Nonetheless the alternative I outlined is better for a number of reasons.

[vsmenon]

Ok. We may want to add an abstraction layer here:

https://github.com/dart-lang/sdk/blob/master/pkg/dev_compiler/tool/input_sdk/patch/developer_patch.dart#L27

(and the other 4-5 call sites). E.g., a hook that allows webdev bootstrap to override, but otherwise falls back on theconsole.debug("dart.developer...", ...) call.

[grouma]

That makes sense to me. Is this something @nshahan should pick up?

[jmesserly]

(unassigned because someone else may pick this up)

FWIW, it was @jacob314 who told me to use console logging, I think because it's easy to subscribe to those events from the Chrome debug protocol. But yeah, since a debugger is listening, we could basically do anything.

What sort of thing is easiest for the debug service to subscribe to? Are there other sorts of events besides console logging?

[jakemac53]

I think adding the ability to provide our own function to call instead of console.debug would be the most straightforward and efficient solution, as opposed to firing events through some other mechanism.

This is mostly only an issue because of how many events flutter fires (one for each frame), if you have an app that does constant animations (like the spinning square) it is really noticeable (and ironically during animations is the worst possible time to be causing jank...).

[jmesserly]

SGTM. The way we've handled this with similar things (such as "ignore whitelisted errors", hot restart) is to have a method on dart. that can be called to set things up. So you could pass in the callback for DDC to use.

Personally, I would remove the "console.debug" calls if they're slow. If the debug service callback isn't registered, then we don't need to log to the console, we can simply do nothing.

[jacob314]

I agree that we should switch off of console.debug for this. The chrome devtools implementation is far slower than I expected. It is strange it hasn't been optimized more.
Sorry for recommending it!

[vsmenon]

Adding to 2.3 for now as a p2. Depending on perf, we can bump up or out.

[vsmenon]

Bumping to 2.4 - if we're hitting perf issues, we can pull forward.

[jmesserly]

Since y'all need to run JS code anyway, feel free to assign functions directly to the developer library object. For example:

let developer = dart_sdk.developer; // note: import Dart's SDK module somehow
// Intercept "dart:developer" inspect...
developer.inspect = function(object) {
  // ... your JS code here ...
  return object;
};

The current definition is simply:

developer.inspect = function(object) {
  console.debug("dart.developer.inspect", object);
  return object;
};

We can provide a cleaner hook for you if needed. Normally I wouldn't advise mucking with SDK internals, but I think the debug service protocol needs to do that anyway, e.g. to find and call extensions.

[vsmenon]

Per offline conversation with @jakemac53 bumping from this milestone. We can override in webdev today as well.