[webkit-dev] Tightening up our include discipline

[Adam Barth]

There shouldn't be any performance costs.  Instead of returning a
boolean (as the virtual function call does today), it will return a
constant AtomicString which can be compared to another AtomicString in
one instruction.

Adam


On Wed, Oct 19, 2011 at 1:31 PM, Sam Weinig <weinig at apple.com> wrote:
> I'd be interested in performance cost to that, especially on a test that
> creates a lot of JS wrapped Events.
> -Sam
> On Oct 18, 2011, at 6:42 PM, Adam Barth wrote:
>
> My thought on that is to use a single virtual function and return an atomic
> string, like we do for elements.
>
> Adam
>
> On Oct 18, 2011 6:10 PM, "Sam Weinig" <weinig at apple.com> wrote:
>>
>> For the specific case of Event, we do need some sort of poor man's RTTI
>> here for the sake of the bindings, but we could consider alternatives.
>>  Since you need to identify the subclasses, you need to either use many
>> virtual functions, as we do currently for Event, or have some tagging
>> mechanism, which we do for classes like Element and EventListener.
>>
>> If we use a tagging technique, we need someway to ensure that the tags are
>> unique. In EventListener, we list the types up front in an enum, though this
>> has same downside as the many virtual functions in that the tendrils of
>> WebAudio reach into the base class.  For Element, we rely on qualified
>> names, which doesn't require the icky reach, but are slightly more costly.
>>  (You still end up with the tendrils in the JS base class).
>>
>> Of course, it might make sense to solve the issue of #ifdefs in Event by
>> simply removing the #ifdefs in Event, since they are just base class
>> implementations. The only downside of that is if we later remove the
>> feature, it is harder to find all the places it touched.
>>
>> -Sam
>>
>> On Oct 18, 2011, at 4:37 PM, Adam Barth wrote:
>>
>> > The other day, Sam chided me on a bug for including a header from
>> > WebCore proper in a file in WebCore/platform.  Even though I know I'm
>> > not supposed to do that, it's an easy mistake to make.  There's some
>> > work going on in https://bugs.webkit.org/show_bug.cgi?id=49192 to
>> > improve the style checker to flag these sorts of bad dependencies.
>> > This check isn't live yet, but don't be surprised if some robotic user
>> > complains about a bad include in one of your patches.  (As always, if
>> > the bot complains incorrectly, please let me know so we can eliminate
>> > false positives.)
>> >
>> > Relatedly, as part of the recent ENABLE cleanup effort, I noticed that
>> > some files (e.g.,
>> > http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Event.cpp)
>> > attract lots of ifdefs because many unrelated features need to
>> > register themselves.  I was thinking it might be an improvement to
>> > restructure things a bit so that, for example, WebAudio wouldn't need
>> > to reach its fingers into Event.cpp and instead could be better
>> > contained in the WebCore/webaudio directory.
>> >
>> > I welcome any thoughts you have on either of these topics.
>> >
>> > Thanks,
>> > Adam
>> > _______________________________________________
>> > webkit-dev mailing list
>> > webkit-dev at lists.webkit.org
>> > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>
>
>