1060788 - Inefficient function pointer in HttpAsyncAborter

Status: NEW

(Core :: Networking, defect, P5)

Description

[[e-mail]]

HttpAsyncAborter uses the curiously recurring template pattern. This means that the type over which it is templated isn't fully defined at the point the template needs to be instantiated. In particular, MSVC is forced to use the worst-case member pointer because it doesn't know yet whether the templated class uses virtual inheritance.

On 32-bit Windows the size of the worst-case member pointer is 16 bytes, and the best case applicable here is 8 bytes (since the actual classes use multiple inheritance). (Ironically if it did know that the templated class used virtual inheritance then it still would only use a 12 byte member pointer.)

It looks as if this could be avoided by making HttpAsyncAborter a member rather than a base class (gfxPlatform does it this way) although it would make the call sites uglier.

The code also seems to be confused about casting member pointers. Since at least C++03, pointers to member functions can be cast from a base class to a derived class. In psuedocode, if D is derived from B, then ((B*)pd)->*mpb is the same as pd->*(D::*)mpb.

Comment 1

[[e-mail]]

Actually come to think of it if you change it from a base class to a member then you need to implement HandleAsyncAbort on the class itself to call the method on the member.

Comment 2

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 3

[Junior [inactive]]

HttpAsyncAborter::mCallOnResume is a std::function instead of member function pointer. We might inline HttpAsyncAborter into HttpBaseChannel. I'm not sure if we have performance win or lose but it could be more cleaner.