Bug 31930 – We don't need 270k memory to determine the vptrs

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Zoltan Herczeg]

I think it is a typo.

[Zoltan Herczeg]

Created attachment 43946 [details]
Allocation fix

[Gavin Barraclough]

Comment on attachment 43946 [details]
Allocation fix

Lol, no, I don't think this should be taking 270k!  Nice catch.

r+ with one comment, since you're no longer calling fastMalloc could you also please delete the comment saying:

// Bizarrely, calling fastMalloc here is faster than allocating space on the stack.

cheers,
G.

[Zoltan Horvath]

Committed r51457: <http://trac.webkit.org/changeset/51457>

[Zoltan Herczeg]

Created attachment 44011 [details]
Patch update (by Sam Weinig)

Additional changes suggested by Sam. (He probably meant COMPILE_ASSERT since STATIC_ASSERT does not exists) Should be reviewed by him.

[Adam Barth]

style-queue ran check-webkit-style on attachment 44011 [details] without any errors.

[Eric Seidel (no email)]

Comment on attachment 44011 [details]
Patch update (by Sam Weinig)

The speedup is only in debug builds I'm sure.

[Eric Seidel (no email)]

I can't tell if this bug should be closed or not.  I reviewed an r? pach on it, but I'm not sure if it should have been closed in the first place.

[Zoltan Herczeg]

(In reply to comment #7)
> I can't tell if this bug should be closed or not.  I reviewed an r? pach on it,
> but I'm not sure if it should have been closed in the first place.

The first patch was the true patch. After it was landed, Sam asked me to change ASSERT-s to COMPILE_ASSERTs, and he was curious also about the gain of that patch. Thank you for reviewing, but I still feel he should review it, since he asked for these changes (unfortunately I can't specify the name of the reviewer anymore). (Hmm, if the changelog is not clear, perhaps I should make it more descriptive.)

[Eric Seidel (no email)]

If you want patches reviewed, you need to re-open the bug though.  Re-opening the bug.

[Eric Seidel (no email)]

Comment on attachment 43946 [details]
Allocation fix

Obsoleting this patch, it was landed as r51457.

[Eric Seidel (no email)]

@Zoltan: Ping?

[Eric Seidel (no email)]

Comment on attachment 44011 [details]
Patch update (by Sam Weinig)

Oh, never mind, This Zoltan is not the same as [e-mail].  queueing for auto-commit.

[WebKit Commit Bot]

Comment on attachment 44011 [details]
Patch update (by Sam Weinig)

Clearing flags on attachment: 44011

Committed r51928: <http://trac.webkit.org/changeset/51928>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.