Bug 13383 - [js-collector-tweaks] Allocate collector blocks aligned
Summary: [js-collector-tweaks] Allocate collector blocks aligned
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: 13382
Blocks: 13384 13389
  Show dependency treegraph
 
Reported: 2007-04-18 00:08 PDT by Maciej Stachowiak
Modified: 2007-04-22 21:23 PDT (History)
0 users

See Also:


Attachments
03-js-gc-aligned-blocks.patch.txt (4.64 KB, patch)
2007-04-18 00:08 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 00:08:17 PDT
Collector blocks should be allocated aligned to their natural size. This will enable future optimizations.
Comment 1 Maciej Stachowiak 2007-04-18 00:08:55 PDT
Created attachment 14064 [details]
03-js-gc-aligned-blocks.patch.txt
Comment 2 Darin Adler 2007-04-18 11:05:31 PDT
Comment on attachment 14064 [details]
03-js-gc-aligned-blocks.patch.txt

Some style nitpicks:

+    uintptr_t ptr = reinterpret_cast<uintptr_t>(result);

This could be a static_cast, because you can cast to/from void* to another pointer type without reinterpret_cast.

+    if (adjust > 0)
+        munmap(reinterpret_cast<void*>(ptr), adjust);
+
+    if (adjust < extra)
+        munmap(reinterpret_cast<void*>(ptr + adjust + BLOCK_SIZE), extra - adjust);
+
+    ptr += adjust;
+    memset(reinterpret_cast<void*>(ptr), 0, BLOCK_SIZE);

I don't think these need casts at all. You can past any pointer type to void* as long as it's not a pointer to const or volatile.
Comment 3 Maciej Stachowiak 2007-04-18 14:58:40 PDT
(In reply to comment #2)
> (From update of attachment 14064 [details] [edit])
> Some style nitpicks:
> 
> +    uintptr_t ptr = reinterpret_cast<uintptr_t>(result);
> 
> This could be a static_cast, because you can cast to/from void* to another
> pointer type without reinterpret_cast.

uintptr_t is not a pointer type, it's an arithmetic type. (Unsigned integer type large enough to hold all the bits of a pointer).

> 
> +    if (adjust > 0)
> +        munmap(reinterpret_cast<void*>(ptr), adjust);
> +
> +    if (adjust < extra)
> +        munmap(reinterpret_cast<void*>(ptr + adjust + BLOCK_SIZE), extra -
> adjust);
> +
> +    ptr += adjust;
> +    memset(reinterpret_cast<void*>(ptr), 0, BLOCK_SIZE);
> 
> I don't think these need casts at all. You can past any pointer type to void*
> as long as it's not a pointer to const or volatile.

Ditto here.

Comment 4 Maciej Stachowiak 2007-04-22 21:23:29 PDT
Landed. I renamed "ptr" to "address" to make more clear that it is a numeric address value, not a pointer.