Sdk: DDC / DDK Closures contain a massive amount of state which severely impacts debugging

[grouma]

This issue was brought to my attention through https://github.com/dart-lang/webdev/issues/800

Reproduction steps:

1) Build and serve the following application with DDC, e.g. webdev serve.

import 'dart:async';

Future<int> recurse(int count) async {
  if (count == 0) {
    return count;
  }
  return recurse(count - 1);
}

void main() async {
  await recurse(100);
}

2) Using Chrome DevTools set a breakpoint on the line that contains return count;.

3) Once the breakpoint is hit, try to step over. Notice that stepping takes several seconds when it should be nearly instant.

Root cause:

The issue is caused by the captured Closure context for a runBody call inside the Dart SDK. These contexts contain tons and tons of constant symbols, for example:

When debugging, this information is serialized and sent to the Chrome debugger (or package:dwds). This causes significant slow downs. We should be able to collect these top level symbols into a single map which resolve this issue.

[vsmenon]

fyi - @sigmundch @rakudrama on thoughts

This may not be easy to do and / or impact performance. The state in question here is in the top-level closure - i.e., the closure for the sdk module in this case.

We hoist computations up into this level for performance reasons (i.e., during execution).

That includes symbols used throughout:

  const $setAll = dartx.setAll = Symbol("dartx.setAll");
  const $removeLast = dartx.removeLast = Symbol("dartx.removeLast");
  ...

and generic types:

  let Uint8ListToNull = () => (Uint8ListToNull = dart.constFn(dart.fnType(core.Null, [typed_data.Uint8List])))();
  let RandomAccessFileTovoid = () => (RandomAccessFileTovoid = dart.constFn(dart.fnType(dart.void, [io.RandomAccessFile])))();
  let dynamicAnddynamicTovoid = () => (dynamicAnddynamicTovoid = dart.constFn(dart.fnType(dart.void, [dart.dynamic, dart.dynamic])))();
  let FutureOfRandomAccessFile = () => (FutureOfRandomAccessFile = dart.constFn(async.Future$(io.RandomAccessFile)))();

Some options:

• Revisit the impact of this hoisting.
• Eliminate some redundancies (there are some I think).
• Get smarter about what we hoist.

[grouma]

I did some further investigation yesterday evening. The massive Closure context is definitely impacting debugging performance. I don't believe the context is actually being serialized to the Chrome debugger until requested though. In light of that, I don't know if there's much we can do. Even collecting the constants into a map may not be beneficial. I'll have to brainstorm with @alan-knight. It also may be worth while to reach out to the Chrome team. Firefox doesn't experience the same issues so my guess is that Chrome is tracking significantly more details.

[vsmenon]

@grouma - is it easy to get the number of symbols you're seeing to sanity check?

Some static slicing and dicing below. The lets are hoisted types. The consts are private member names.

> grep '^  let' dart_sdk.js | wc
    1054    5654   74188
> grep '^  const' dart_sdk.js | wc
   10042   50432  693799
> grep '^  const' dart_sdk.js | grep dartx | wc
    2864   17182  208955
> grep '^  const' dart_sdk.js | grep privateName | wc
    4215   21074  334190

[grouma]

I reached out to the Chrome team. Looks like we hit a known issue that is high on their TODO list. They intend to address the issue in Q4. I'll link details when I get them.

[grouma]

Tracking issue on the Chrome side: https://bugs.chromium.org/p/v8/issues/detail?id=9938

[vsmenon]

Thanks. I did land a minor change to reduce the number of symbols ( https://dart-review.googlesource.com/c/sdk/+/123922 ) by ~20%, but we'll need the Chrome fix to really fix this.

[DanTup]

Does anyone know what the status of this is? The issue linked above is tagged with "merged-7.9" which I presume is a V8 version. This list suggests V8 7.9 was included in Chrome 79. However Chrome stable is v80 and the performance still seems very bad in my testing.

[grouma]

I tested the Chrome fix on Canary a while back. It improved the situation but did not completely resolve the issue. We discussed internally that to fully resolve the issue would require a restructure of our SDK.

@sigmundch @vsmenon @Markzipan want to chime in a provide any updates or plans

[Markzipan]

The simplest and quickest DDC change would be to limit top-level symbols emitted by the compiler by lazily loading them or adding some other form of indirection. We haven't discussed many more calculated changes - such as emitting fewer/unused symbols or using a different scheme entirely - though.

[sigmundch]

I'm tentatively closing this issue since @Markzipan landed 37d8c78 and 2e97d91 to undo the overhead of hoisting so many symbols on each module.