Bug 297124
| Summary: | [Bindings] Optimize union IDL attribute setters by avoiding conversion to variant | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Sam Weinig <sam> |
| Component: | Bindings | Assignee: | Sam Weinig <sam> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | cdumez, webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | Safari 18 | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Sam Weinig
Setters for IDL attributes with a union type can be optimized by taking advantage of the fact that we type check the JS value, at which point we know the concrete final type, but then construct a variant with that value, at which which point we lose that information. This means, the first thing the implementation often does is switch on the variant it was passed to recover that information. Instead, the implementation can expose overloads for each member of the union, thus preserving the information.
For example, for CanvasFillStrokeStyles's strokeStyle attribute:
```
interface mixin CanvasFillStrokeStyles {
// colors and styles (see also the CanvasPathDrawingStyles and CanvasTextDrawingStyles interfaces)
attribute (DOMString or CanvasGradient or CanvasPattern) strokeStyle; // (default black)
...
}
```
The implementation currently exposes a getter and setter:
```
using StyleVariant = Variant<String, RefPtr<CanvasGradient>, RefPtr<CanvasPattern>>;
StyleVariant strokeStyle() const;
void setStrokeStyle(StyleVariant&&);
```
and what setStrokeStyle has to do is check what type it got and then do something with the style:
```
void CanvasRenderingContext2DBase::setStrokeStyle(CanvasRenderingContext2DBase::StyleVariant&& style)
{
if (std::holds_alternative<String>(style)) {
...
} else if (std::holds_alternative<RefPtr<CanvasGradient>>(style)) {
...
} else {
...
}
```
Instead, it will now expose 3 setters:
```
using StyleVariant = Variant<String, RefPtr<CanvasGradient>, RefPtr<CanvasPattern>>;
StyleVariant strokeStyle() const;
void setStrokeStyle(String&&);
void setStrokeStyle(RefPtr<CanvasGradient>&&);
void setStrokeStyle(RefPtr<CanvasPattern>&&);
```
with no additional checking needed.
(This idea originates from Kimmo Kinnunen, all credit to them)
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Sam Weinig
Pull request: https://github.com/WebKit/WebKit/pull/49126
Radar WebKit Bug Importer
<rdar://problem/158423078>
EWS
Committed 301035@main (16afa35bbfd9): <https://commits.webkit.org/301035@main>
Reviewed commits have been landed. Closing PR #49126 and removing active labels.