Bug 13389 - [js-collector-tweaks] move mark and collectOnMainThreadOnly bits into separate bitmaps
Summary: [js-collector-tweaks] move mark and collectOnMainThreadOnly bits into separat...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Maciej Stachowiak
URL:
Keywords:
Depends on: 13381 13382 13383 13384 13385 13386 13387 13388
Blocks: 13390
  Show dependency treegraph
 
Reported: 2007-04-18 01:01 PDT by Maciej Stachowiak
Modified: 2007-04-23 15:14 PDT (History)
0 users

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Stachowiak 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.
Comment 1 Maciej Stachowiak 2007-04-18 01:01:33 PDT
Created attachment 14070 [details]
09-js-gc-mark-bitmap+32byte-cell.patch.txt
Comment 2 Darin Adler 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.
Comment 3 Maciej Stachowiak 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?
Comment 4 Geoffrey Garen 2007-04-18 21:07:39 PDT
Code looks right to me, modulo darin's comments.
Comment 5 Maciej Stachowiak 2007-04-23 12:03:39 PDT
Created attachment 14146 [details]
fix the sys/mman.h thing, plus some other cleanups
Comment 6 Maciej Stachowiak 2007-04-23 13:17:21 PDT
Created attachment 14148 [details]
actually fixed the mman thing
Comment 7 Darin Adler 2007-04-23 15:05:31 PDT
Comment on attachment 14148 [details]
actually fixed the mman thing

r=me