Bug 13389

Summary:

[js-collector-tweaks] move mark and collectOnMainThreadOnly bits into separate bitmaps

Product:

WebKit

Reporter:

Maciej Stachowiak <mjs>

Component:

JavaScriptCore

Assignee:

Maciej Stachowiak <mjs>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

Version:

523.x (Safari 3)

Hardware:

Mac

OS:

OS X 10.4

Bug Depends on:

13381, 13382, 13383, 13384, 13385, 13386, 13387, 13388

Bug Blocks:

13390

Attachments:

Description	Flags
09-js-gc-mark-bitmap+32byte-cell.patch.txt	darin: review-
fix the sys/mman.h thing, plus some other cleanups	none
actually fixed the mman thing	darin: review+

Maciej Stachowiak

Reported 2007-04-18 01:01:06 PDT

Move mark and collectOnMainThreadOnly bits into separate bitmaps. This saves 4 bytes per cell, allowing shrink of cell size to 32, which leads to a .8% speed improvement on iBench.

Attachments
09-js-gc-mark-bitmap+32byte-cell.patch.txt (14.50 KB, patch) 2007-04-18 01:01 PDT, Maciej Stachowiak	darin: review-	Details Formatted Diff Diff
fix the sys/mman.h thing, plus some other cleanups (14.86 KB, patch) 2007-04-23 12:03 PDT, Maciej Stachowiak	no flags	Details Formatted Diff Diff
actually fixed the mman thing (14.74 KB, patch) 2007-04-23 13:17 PDT, Maciej Stachowiak	darin: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Maciej Stachowiak

Comment 1 2007-04-18 01:01:33 PDT

Created attachment 14070 [details] 09-js-gc-mark-bitmap+32byte-cell.patch.txt

Darin Adler

Comment 2 2007-04-18 11:30:40 PDT

Comment on attachment 14070 [details] 09-js-gc-mark-bitmap+32byte-cell.patch.txt +#include <sys/mman.h> I don't think it's right that have this outside an #if -- is it really a header that exists on all JavaScriptCore platforms. Now that all these constants have to be moved into a header file, maybe we should make them have non-all-capitals name or use a namespace or something. They're pretty ugly.

Maciej Stachowiak

Comment 3 2007-04-18 15:06:30 PDT

(In reply to comment #2) > (From update of attachment 14070 [details] [edit]) > +#include <sys/mman.h> > > I don't think it's right that have this outside an #if -- is it really a header > that exists on all JavaScriptCore platforms. OK, I'll put it in an ifdef (I think it is only needed for non-Darwin Unix platforms, to get mmap/munmap). > Now that all these constants have to be moved into a header file, maybe we > should make them have non-all-capitals name or use a namespace or something. > They're pretty ugly. I'm happy to give them more normal names, but maybe that should be a separate patch?

Geoffrey Garen

Comment 4 2007-04-18 21:07:39 PDT

Code looks right to me, modulo darin's comments.

Maciej Stachowiak

Comment 5 2007-04-23 12:03:39 PDT

Created attachment 14146 [details] fix the sys/mman.h thing, plus some other cleanups

Maciej Stachowiak

Comment 6 2007-04-23 13:17:21 PDT

Created attachment 14148 [details] actually fixed the mman thing

Darin Adler

Comment 7 2007-04-23 15:05:31 PDT

Comment on attachment 14148 [details] actually fixed the mman thing r=me

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