Bug 208367 – Add document about WTF malloc-related macros

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Yusuke Suzuki]

Like, WTF_MAKE_FAST_ALLOCATED, WTF_MAKE_ISO_ALLOCATED etc.

[Yusuke Suzuki]

Created attachment 397032 [details]
Patch

[Yusuke Suzuki]

Created attachment 397033 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 397033 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397033&action=review

> Source/WTF/wtf/FastMalloc.h:31
> +// allocation is handled bmalloc if bmalloc is available.

handled by bmalloc

> Source/WTF/wtf/FastMalloc.h:35
> +//   1. If your class / struct is derived one of the base class using WTF_MAKE_ISO_ALLOCATED / WTF_MAKE_ISO_ALLOCATED_EXPORT,

derived from a base class which uses

> Source/WTF/wtf/FastMalloc.h:42
> +// Let's explain the difference in details.

Let's explain the differences in detail.

> Source/WTF/wtf/FastMalloc.h:46
> +//     If you annotate class / struct with these macros, class / struct is allocated with FastMalloc (bmalloc if it is available).

I don't think you need to say "If you annotate class / struct with these macros"

> Source/WTF/wtf/FastMalloc.h:49
> +//     WTF_MAKE_FAST_ALLOCATED is for class and WTF_MAKE_STRUCT_FAST_ALLOCATED is for struct. The difference between them are how we

WTF_MAKE_FAST_ALLOCATED is for classes and WTF_MAKE_STRUCT_FAST_ALLOCATED is for structs

> Source/WTF/wtf/FastMalloc.h:54
> +//     If you annotate class / struct with these macros, in release build, this is completely the same to WTF_MAKE_FAST_ALLOCATED etc.

Normally, these are identical to WTF_MAKE_FAST_ALLOCATED.

> Source/WTF/wtf/FastMalloc.h:55
> +//     These macros act differently when MallocHeapBreakdown debugging feature is enabled. When this feature is enabled, bmalloc is switched

When the MallocHeapBreakdown debugging feature

> Source/WTF/wtf/FastMalloc.h:56
> +//     to using System Malloc internally, and bmalloc creates MallocZone per this annotation. MallocZone allows us to track heap usage

creates a named MallocZone

[Yusuke Suzuki]

Comment on attachment 397033 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397033&action=review

Thanks

>> Source/WTF/wtf/FastMalloc.h:31
>> +// allocation is handled bmalloc if bmalloc is available.
> 
> handled by bmalloc

Fixed.

>> Source/WTF/wtf/FastMalloc.h:35
>> +//   1. If your class / struct is derived one of the base class using WTF_MAKE_ISO_ALLOCATED / WTF_MAKE_ISO_ALLOCATED_EXPORT,
> 
> derived from a base class which uses

Fixed.

>> Source/WTF/wtf/FastMalloc.h:42
>> +// Let's explain the difference in details.
> 
> Let's explain the differences in detail.

Fixed.

>> Source/WTF/wtf/FastMalloc.h:46
>> +//     If you annotate class / struct with these macros, class / struct is allocated with FastMalloc (bmalloc if it is available).
> 
> I don't think you need to say "If you annotate class / struct with these macros"

Removed.

>> Source/WTF/wtf/FastMalloc.h:49
>> +//     WTF_MAKE_FAST_ALLOCATED is for class and WTF_MAKE_STRUCT_FAST_ALLOCATED is for struct. The difference between them are how we
> 
> WTF_MAKE_FAST_ALLOCATED is for classes and WTF_MAKE_STRUCT_FAST_ALLOCATED is for structs

Fixed.

>> Source/WTF/wtf/FastMalloc.h:54
>> +//     If you annotate class / struct with these macros, in release build, this is completely the same to WTF_MAKE_FAST_ALLOCATED etc.
> 
> Normally, these are identical to WTF_MAKE_FAST_ALLOCATED.

Fixed.

>> Source/WTF/wtf/FastMalloc.h:55
>> +//     These macros act differently when MallocHeapBreakdown debugging feature is enabled. When this feature is enabled, bmalloc is switched
> 
> When the MallocHeapBreakdown debugging feature

Fixed.

>> Source/WTF/wtf/FastMalloc.h:56
>> +//     to using System Malloc internally, and bmalloc creates MallocZone per this annotation. MallocZone allows us to track heap usage
> 
> creates a named MallocZone

Fixed.

[Yusuke Suzuki]

Created attachment 397037 [details]
Patch

[Darin Adler]

Comment on attachment 397037 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397037&action=review

I love the idea of having comments. The shorter and simpler this can be, the better. Already good.

> Source/WTF/wtf/FastMalloc.h:37
> +//   2. If your class / struct is JavaScript exposed one, use WTF_MAKE_ISO_ALLOCATED.

I don’t know what "JavaScript exposed" means. Maybe we could be more specific and mechanical on how we can tell if a class or struct is "JavaScript exposed"?

> Source/WTF/wtf/FastMalloc.h:47
> +//     Because of performance and memory footprint, we encourage using FastMalloc for all the class / struct allocations if possible.

The issue is that if we don’t use WTF_MAKE_FAST_ALLOCATED then new/delete will use operator new/delete, which will use system malloc. And we want to minimize the use of system malloc, because bmalloc offers better performance and memory footprint. But that’s not stated here.

[Yusuke Suzuki]

Comment on attachment 397037 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397037&action=review

>> Source/WTF/wtf/FastMalloc.h:37
>> +//   2. If your class / struct is JavaScript exposed one, use WTF_MAKE_ISO_ALLOCATED.
> 
> I don’t know what "JavaScript exposed" means. Maybe we could be more specific and mechanical on how we can tell if a class or struct is "JavaScript exposed"?

I should rewrite it with `DOM object`, maybe this is much better to clarify this.

>> Source/WTF/wtf/FastMalloc.h:47
>> +//     Because of performance and memory footprint, we encourage using FastMalloc for all the class / struct allocations if possible.
> 
> The issue is that if we don’t use WTF_MAKE_FAST_ALLOCATED then new/delete will use operator new/delete, which will use system malloc. And we want to minimize the use of system malloc, because bmalloc offers better performance and memory footprint. But that’s not stated here.

Nice, I've noted about it.

[Yusuke Suzuki]

Committed r260409: <https://trac.webkit.org/changeset/260409>

[Radar WebKit Bug Importer]

<rdar://problem/62088206>