Bug 207618 – Shrink CachedResource

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

Shrink CachedResource

[Yusuke Suzuki]

Created attachment 390499 [details]
Patch

[Yusuke Suzuki]

Created attachment 390500 [details]
Patch

[Mark Lam]

Comment on attachment 390500 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        For each enum class, we define `bitsOfXXX` value, which indicates # of bits to represent it. And using

Let's replace bitsOfXXX with XXXBitWidth to be consistent with https://bugs.webkit.org/show_bug.cgi?id=207616.

> Source/WTF/wtf/Markable.h:150
> +    Optional<T> asOptional() const
> +    {
> +        if (bool(*this))
> +            return m_value;
> +        return WTF::nullopt;
> +    }

This is identical to the cast operator above.  Can you implement this in terms of the cast operator or better yet, implement the cast operator in term of asOptional()?

> Source/WebCore/loader/ResourceLoaderOptions.h:49
> +static constexpr unsigned bitsOfSendCallbackPolicy = 1;

Let's call this sendCallbackPolicyBitWidth.

> Source/WebCore/loader/ResourceLoaderOptions.h:57
> +static constexpr unsigned bitsOfContentSniffingPolicy = 1;

Ditto: contentSniffingPolicyBitWidth.

> Source/WebCore/loader/ResourceLoaderOptions.h:63
> +static constexpr unsigned bitsOfDataBufferingPolicy = 1;

Ditto here and the others below.

> Source/WebCore/loader/cache/CachedResource.h:94
> +    static constexpr unsigned bitsOfType = 5;
> +    static_assert(static_cast<unsigned>(Type::LastType) <= ((1U << bitsOfType) - 1));

nit: Let's call this typeBitWidth instead.

> Source/WebCore/loader/cache/CachedResource.h:104
> +    static constexpr unsigned bitsOfStatus = 3;
> +    static_assert(static_cast<unsigned>(DecodeError) <= ((1ULL << bitsOfStatus) - 1));

Ditto: statusBitWidth.

> Source/WebCore/loader/cache/CachedResource.h:150
> +    static constexpr unsigned bitsOfPreloadResult = 2;

Ditto: preloadResultBitWidth.

> Source/WebCore/platform/network/ResourceLoadPriority.h:40
> +static constexpr unsigned bitsOfResourceLoadPriority = 3;

Ditto: resourceLoadPriorityBitWidth.

> Source/WebCore/platform/network/ResourceResponseBase.h:51
> +    static constexpr unsigned bitsOfType = 3;

Ditto: typeBitWidth.

> Source/WebCore/platform/network/ResourceResponseBase.h:53
> +    static constexpr unsigned bitsOfTainting = 2;

Ditto: taintingBitWidth.

> Source/WebCore/platform/network/StoredCredentialsPolicy.h:35
> +static constexpr unsigned bitsOfStoredCredentialsPolicy = 2;

Ditto: storedCredentialsPolicyBitWidth.

[Yusuke Suzuki]

Comment on attachment 390500 [details]
Patch

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

Thanks!

>> Source/WebCore/ChangeLog:9
>> +        For each enum class, we define `bitsOfXXX` value, which indicates # of bits to represent it. And using
> 
> Let's replace bitsOfXXX with XXXBitWidth to be consistent with https://bugs.webkit.org/show_bug.cgi?id=207616.

Talked with Mark, replacing it with bitWidthOfXXX.

>> Source/WTF/wtf/Markable.h:150
>> +    }
> 
> This is identical to the cast operator above.  Can you implement this in terms of the cast operator or better yet, implement the cast operator in term of asOptional()?

OK, implementing it via cast operator.

>> Source/WebCore/loader/ResourceLoaderOptions.h:49
>> +static constexpr unsigned bitsOfSendCallbackPolicy = 1;
> 
> Let's call this sendCallbackPolicyBitWidth.

Fixed.

>> Source/WebCore/loader/ResourceLoaderOptions.h:57
>> +static constexpr unsigned bitsOfContentSniffingPolicy = 1;
> 
> Ditto: contentSniffingPolicyBitWidth.

Fixed.

>> Source/WebCore/loader/ResourceLoaderOptions.h:63
>> +static constexpr unsigned bitsOfDataBufferingPolicy = 1;
> 
> Ditto here and the others below.

Fixed.

>> Source/WebCore/loader/cache/CachedResource.h:94
>> +    static_assert(static_cast<unsigned>(Type::LastType) <= ((1U << bitsOfType) - 1));
> 
> nit: Let's call this typeBitWidth instead.

Fixed.

>> Source/WebCore/loader/cache/CachedResource.h:104
>> +    static_assert(static_cast<unsigned>(DecodeError) <= ((1ULL << bitsOfStatus) - 1));
> 
> Ditto: statusBitWidth.

Fixed.

>> Source/WebCore/loader/cache/CachedResource.h:150
>> +    static constexpr unsigned bitsOfPreloadResult = 2;
> 
> Ditto: preloadResultBitWidth.

Fixed.

>> Source/WebCore/platform/network/ResourceLoadPriority.h:40
>> +static constexpr unsigned bitsOfResourceLoadPriority = 3;
> 
> Ditto: resourceLoadPriorityBitWidth.

Fixed.

>> Source/WebCore/platform/network/ResourceResponseBase.h:51
>> +    static constexpr unsigned bitsOfType = 3;
> 
> Ditto: typeBitWidth.

Fixed.

>> Source/WebCore/platform/network/ResourceResponseBase.h:53
>> +    static constexpr unsigned bitsOfTainting = 2;
> 
> Ditto: taintingBitWidth.

Fixed.

>> Source/WebCore/platform/network/StoredCredentialsPolicy.h:35
>> +static constexpr unsigned bitsOfStoredCredentialsPolicy = 2;
> 
> Ditto: storedCredentialsPolicyBitWidth.

Fixed.

[Yusuke Suzuki]

Comment on attachment 390500 [details]
Patch

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

> Source/WebCore/loader/cache/CachedImage.h:188
> +    unsigned m_updateImageDataCount : 3;

Initializer of this is added.

[Yusuke Suzuki]

Committed r256482: <https://trac.webkit.org/changeset/256482>

[Radar WebKit Bug Importer]

<rdar://problem/59402466>

[Yusuke Suzuki]

*** Bug 187460 has been marked as a duplicate of this bug. ***