Bug 164096 – [DOM] Add JSEventType

• Component: Bindings
• Status: RESOLVED FIXED

[Yusuke Suzuki]

I've found that Event is frequently touched in Speedometer. And I think it is frequently touched in the real Web.
But, if you touch the accessors of Event interface, you always need to perform jsDynamicCast<Event*>.
It is costly because the typical event instance is not direct instance of the Event class. For example,

MouseEvent -> UIEvent -> Event

Event resides in higher position in the hierarchy. But it has many accessors and they are used (currentTarget, type, target etc.)
So it's worth introducing JSEventType (similar to JSDocumentWrapperType) and jsEventCast.

Current my policy is, "If the jsDynamicCast<XXX>() is so frequently called and it always walks many classes, we should consider introducing some JSType.".
For example, CanvasWebGLContext thing does not require it because jsDynamicCast<....Context>() typically succeeds with the first ClassInfo.
But it is not for Event.

[Yusuke Suzuki]

Created attachment 293086 [details]
Patch

[Yusuke Suzuki]

Created attachment 293089 [details]
Patch

[Yusuke Suzuki]

Created attachment 293091 [details]
Patch

[Yusuke Suzuki]

Created attachment 293092 [details]
Patch

[Yusuke Suzuki]

Committed r208028: <http://trac.webkit.org/changeset/208028>

[Darin Adler]

Comment on attachment 293092 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=293092&action=review

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2531
>      return "jsElementCast" if $interfaceName eq "Element";
>      return "jsDocumentCast" if $interfaceName eq "Document";
>      return "jsEventTargetCast" if $interfaceName eq "EventTarget";
> +    return "jsEventCast" if $interfaceName eq "Event";
>      return "jsDynamicCast<JS$interfaceName*>";

I think we should consider putting all these optimizations into specializations of the jsDynamicCast function. Then we can remove all these special cases from the code generator, and programmers can’t ever make the mistake of using the slow version by accident.

Any reason not to do that?

[Yusuke Suzuki]

(In reply to comment #6)
> Comment on attachment 293092 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=293092&action=review
> 
> > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2531
> >      return "jsElementCast" if $interfaceName eq "Element";
> >      return "jsDocumentCast" if $interfaceName eq "Document";
> >      return "jsEventTargetCast" if $interfaceName eq "EventTarget";
> > +    return "jsEventCast" if $interfaceName eq "Event";
> >      return "jsDynamicCast<JS$interfaceName*>";
> 
> I think we should consider putting all these optimizations into
> specializations of the jsDynamicCast function. Then we can remove all these
> special cases from the code generator, and programmers can’t ever make the
> mistake of using the slow version by accident.
> 
> Any reason not to do that?

Definitely we should do that.
One concern is that we have a bad time if we forget to include the header that specializes jsDynamicCast.
It is serious because it will not cause any compile errors but will cause performance regression.

I think we should add a helper function in WebCore, like the following.

template<typename To, typename From>
ALWAYS_INLINE To jsDowncast(From from)
{
    return jsDynamicCast<To>(from);
}

template<>
template<typename From>
ALWAYS_INLINE jsDowncast<Node*>(From from)
{
    return jsNodeCast(from);
}

[Yusuke Suzuki]

(In reply to comment #7)
> (In reply to comment #6)
> > Comment on attachment 293092 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=293092&action=review
> > 
> > > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2531
> > >      return "jsElementCast" if $interfaceName eq "Element";
> > >      return "jsDocumentCast" if $interfaceName eq "Document";
> > >      return "jsEventTargetCast" if $interfaceName eq "EventTarget";
> > > +    return "jsEventCast" if $interfaceName eq "Event";
> > >      return "jsDynamicCast<JS$interfaceName*>";
> > 
> > I think we should consider putting all these optimizations into
> > specializations of the jsDynamicCast function. Then we can remove all these
> > special cases from the code generator, and programmers can’t ever make the
> > mistake of using the slow version by accident.
> > 
> > Any reason not to do that?
> 
> Definitely we should do that.
> One concern is that we have a bad time if we forget to include the header
> that specializes jsDynamicCast.
> It is serious because it will not cause any compile errors but will cause
> performance regression.
> 
> I think we should add a helper function in WebCore, like the following.
> 
> template<typename To, typename From>
> ALWAYS_INLINE To jsDowncast(From from)
> {
>     return jsDynamicCast<To>(from);
> }
> 
> template<>
> template<typename From>
> ALWAYS_INLINE jsDowncast<Node*>(From from)
> {
>     return jsNodeCast(from);
> }

I've opened the issue. https://bugs.webkit.org/show_bug.cgi?id=164137
And soon I'll upload the patch.