Bug 31930

Summary:

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

Product:

WebKit

Reporter:

Zoltan Herczeg <zherczeg>

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Minor

CC:

abarth, barraclough, commit-queue, eric, sam, zoltan

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

OS X 10.5

Attachments:

Description	Flags
Allocation fix	none
Patch update (by Sam Weinig)	none

Zoltan Herczeg

Reported 2009-11-27 05:42:03 PST

I think it is a typo.

Attachments
Allocation fix (2.35 KB, patch) 2009-11-27 05:48 PST, Zoltan Herczeg	no flags	Details Formatted Diff Diff
Patch update (by Sam Weinig) (2.56 KB, patch) 2009-11-30 01:18 PST, Zoltan Herczeg	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Zoltan Herczeg

Comment 1 2009-11-27 05:48:05 PST

Created attachment 43946 [details] Allocation fix

Gavin Barraclough

Comment 2 2009-11-27 15:26:25 PST

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

Comment 3 2009-11-28 02:31:40 PST

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

Zoltan Herczeg

Comment 4 2009-11-30 01:18:33 PST

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

Comment 5 2009-11-30 12:50:35 PST

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

Eric Seidel (no email)

Comment 6 2009-11-30 21:46:36 PST

Comment on attachment 44011 [details] Patch update (by Sam Weinig) The speedup is only in debug builds I'm sure.

Eric Seidel (no email)

Comment 7 2009-11-30 21:47:41 PST

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

Comment 8 2009-11-30 23:30:58 PST

(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)

Comment 9 2009-12-01 07:11:46 PST

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

Eric Seidel (no email)

Comment 10 2009-12-02 11:59:30 PST

Comment on attachment 43946 [details] Allocation fix Obsoleting this patch, it was landed as r51457.

Eric Seidel (no email)

Comment 11 2009-12-09 14:46:59 PST

@Zoltan: Ping?

Eric Seidel (no email)

Comment 12 2009-12-09 14:48:24 PST

Comment on attachment 44011 [details] Patch update (by Sam Weinig) Oh, never mind, This Zoltan is not the same as zoltan@webkit.org. queueing for auto-commit.

WebKit Commit Bot

Comment 13 2009-12-09 15:01:39 PST

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

Comment 14 2009-12-09 15:01:47 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.