Bug 216104

Summary: [CG] Cache the last status of the image encoded data
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: ImagesAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: peng.liu6, simon.fraser, thorton, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
youennf: review+, ews-feeder: commit-queue-
Patch none

Description Said Abou-Hallawa 2020-09-02 15:48:54 PDT
In one of the WebKit traces, opening a Wikipedia page shows that CGImageSourceCopyPropertiesAtIndex() was called over 500 times. ImageDecoderCG::encodedDataStatus() keeps calling this function as long as the encoded data is not complete. This may happen if the network is slow. This case can be optimized by caching the last status of the image encoded data.
Comment 1 Said Abou-Hallawa 2020-09-02 16:21:12 PDT
Created attachment 407830 [details]
Patch
Comment 2 Peng Liu 2020-09-02 16:46:27 PDT
Comment on attachment 407830 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        can be made with calling system functions.

s/with/without/
Sounds like?
Comment 3 Said Abou-Hallawa 2020-09-02 17:28:03 PDT
Created attachment 407838 [details]
Patch
Comment 4 Radar WebKit Bug Importer 2020-09-09 15:49:15 PDT
<rdar://problem/68601184>
Comment 5 Tim Horton 2020-09-10 11:39:04 PDT
Comment on attachment 407838 [details]
Patch

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

> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:-239
> -        // Ragnaros yells: TOO SOON! You have awakened me TOO SOON, Executus!

This critical historical documentation must remain!
Comment 6 Said Abou-Hallawa 2020-09-10 13:02:11 PDT
Created attachment 408470 [details]
Patch
Comment 7 Said Abou-Hallawa 2020-09-10 13:03:06 PDT
Comment on attachment 407838 [details]
Patch

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

>> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:-239
>> -        // Ragnaros yells: TOO SOON! You have awakened me TOO SOON, Executus!
> 
> This critical historical documentation must remain!

I put it back :)
Comment 8 youenn fablet 2020-09-11 00:07:48 PDT
Comment on attachment 408470 [details]
Patch

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

> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:253
> +            m_encodedDataStatus = EncodedDataStatus::Error;

Could be a one liner:
m_encodedDataStatus = !m_isAllDataReceived ? EncodedDataStatus::Unknown : EncodedDataStatus::Error;

> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:260
>          RetainPtr<CFDictionaryRef> image0Properties = adoptCF(CGImageSourceCopyPropertiesAtIndex(m_nativeDecoder.get(), 0, imageSourceOptions().get()));

auto

> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:266
> +        if (!CFDictionaryContainsKey(image0Properties.get(), kCGImagePropertyPixelWidth) || !CFDictionaryContainsKey(image0Properties.get(), kCGImagePropertyPixelHeight)) {

You could group both this if and previous if in a single statement.
Comment 9 Said Abou-Hallawa 2020-09-14 02:41:43 PDT
Created attachment 408694 [details]
Patch
Comment 10 EWS 2020-09-14 09:22:35 PDT
Committed r267016: <https://trac.webkit.org/changeset/267016>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408694 [details].