Bug 13383 – [js-collector-tweaks] Allocate collector blocks aligned

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

[Maciej Stachowiak]

Collector blocks should be allocated aligned to their natural size. This will enable future optimizations.

[Maciej Stachowiak]

Created attachment 14064 [details]
03-js-gc-aligned-blocks.patch.txt

[Darin Adler]

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.

[Maciej Stachowiak]

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

[Maciej Stachowiak]

Landed. I renamed "ptr" to "address" to make more clear that it is a numeric address value, not a pointer.