Request to optimize js_util.callMethod when using const args (dart2js).

[ferhatb]

Ran across while trying to optimize very low level Flutter web class.

int someValue = 3453;
js_util.callMethod(someValue, 'toString', const <int>[16]);

instead of generating. someValue.toString(16); , checks type of someValue and calls someValue.apply(....) with array.

[rakudrama]

js_util.callMethod is discussed at length at #39740.
The conclusion was that we should make it possible to use package:js from within the SDK. Then @JS() classes could be used for most cases, with no apply and conversion of arguments.
It is difficult to justify heavily optimizing js_util.callMethod if similar goals can be achieved with @JS.

The example here is a little different as the receiver is a primitive type, which might not be supported via @JS.

For the specific case of int.toRadixString, it might be possible to improve the performance.
Can you provide a profile showing the line-by-line costs of toRadixString$1 in your application?

[ferhatb]

@rakudrama, we convert Flutter Color to css color for canvas apis quite a bit.

to profile , run the following colorToCssString(const Color(0xFF00FFFF) in a tight loop:
You can also replace ui.Color with int to run this outside flutter context.

/// Converts color to a css compatible attribute value.
String colorToCssString(ui.Color color) {
  if (color == null) {
    return null;
  }
  final int value = color.value;
  if ((0xff000000 & value) == 0xff000000) {
    return _colorToCssStringRgbOnly(color);
  } else {
    final double alpha = ((value >> 24) & 0xFF) / 255.0;
    final StringBuffer sb = StringBuffer();
    sb.write('rgba(');
    sb.write(((value >> 16) & 0xFF).toString());
    sb.write(',');
    sb.write(((value >> 8) & 0xFF).toString());
    sb.write(',');
    sb.write((value & 0xFF).toString());
    sb.write(',');
    sb.write(alpha.toString());
    sb.write(')');
    return sb.toString();
  }
}

/// Returns the CSS value of this color without the alpha component.
///
/// This is useful when painting shadows as on the Web shadow opacity combines
/// with the paint opacity.
String _colorToCssStringRgbOnly(ui.Color color) {
  final int value = color.value;
  final String paddedValue = '00000${value.toRadixString(16)}';
  return '#${paddedValue.substring(paddedValue.length - 6)}';
}

[rakudrama]

I did some investigation and we discussed this off-line.

The performance characteristics of this code on Chrome is dominated by

1. the JavaScript call to .toString(radix), which appears to take time proportional to the number of digits.
2. and (to a lesser extent) of the JavaScript call to .substring.

The overhead of the extra checks on the path in the compiled code contribute relatively little to the run time (<5%)

The original code calls toRadixString on a number of the form 0xFFxxxxxx. This generates eight digits. The string interpolation compiles to string concatenation of five zeros with the eight digits, generating a string of length 13. 13 is a magic number in V8 - it is the smallest size string that is allocated as a cons-string. Taking a substring of a cons-string appears to be more expensive that a smaller flat string.

The referenced change flutter/engine#17866 gains 25% mostly by masking the input to .toString(16) to generate 25% fewer digits.

These performance characteristics to be peculiar to V8. JSC (Safari) and SM (Firefox) do better.

\	V8	JSC	SM
version1	0.36	0.21	0.24
version2	0.27	0.18	0.23
version3	0.23	0.17	0.09

• version1 is the code as presented (8 digits, concat, substring)
• version2 is "#${color.value.toRadixString(16).substring(2)}", i.e. simplified knowing that formatted integer is always 8 digits, so the last six can be taken simply.
• version3 is the same as version1, but masking with integer with 0xFFFFFF.

@verwaest @mathiasbynens @hannespayer V8 is slower than JSC and SM. We would love to get the 2x better performance that SM shows is possible since this code is hot in some Flutter web apps.

[hannespayer]

Thanks for reporting!

@LeszekSwirski will have a look.

[LeszekSwirski]

The 13 magic number thing is kind-of a KI, but all heuristics are bad at some point 🤷. I think asking devs to avoid unnecessary concatenations is a fair here.

Looks like for radix 16 we do fall down the Number.prototype.toString slow path, which we probably don't have to for small integers. I've filed a V8 bug for it: https://crbug.com/v8/10477

As an aside, you may also consider a version 2+3, where you mask the integer with 0x1FFFFFF to maintain a known prefix (like in v2) but allow the value to be a small tagged integer (like in v3).