Bug 222411 - WebGL context count is not limited for GPU process
Summary: WebGL context count is not limited for GPU process
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
: 236964 (view as bug list)
Depends on:
Blocks: webglgpup 238280
  Show dependency treegraph
 
Reported: 2021-02-25 03:49 PST by Kimmo Kinnunen
Modified: 2022-03-23 13:17 PDT (History)
16 users (show)

See Also:


Attachments
Patch (30.31 KB, patch)
2022-03-02 06:09 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (30.28 KB, patch)
2022-03-02 06:14 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (30.19 KB, patch)
2022-03-02 06:35 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (30.36 KB, patch)
2022-03-02 08:45 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (30.46 KB, patch)
2022-03-03 02:57 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch for landing (30.46 KB, patch)
2022-03-03 03:11 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kimmo Kinnunen 2021-02-25 03:49:48 PST
WebGL context count is not limited for GPU process

Without GPU process, we have 20 context limit per web process.
With GPU process, we don't have any limit.


webgl/many-contexts-access-after-loss.html [ Failure ]
webgl/max-active-contexts-console-warning.html [ Failure ]
webgl/max-active-contexts-gc.html [ Failure ]
webgl/max-active-contexts-oldest-context-lost.html [ Failure ]
webgl/max-active-contexts-webglcontextlost-prevent-default.html [ Timeout ]
Comment 1 Radar WebKit Bug Importer 2021-03-04 01:35:09 PST
<rdar://problem/75025960>
Comment 2 Kimmo Kinnunen 2022-03-02 06:09:39 PST
Created attachment 453597 [details]
Patch
Comment 3 Kimmo Kinnunen 2022-03-02 06:14:15 PST
Created attachment 453599 [details]
Patch
Comment 4 Kimmo Kinnunen 2022-03-02 06:35:14 PST
Created attachment 453601 [details]
Patch
Comment 5 Kimmo Kinnunen 2022-03-02 08:45:17 PST
Created attachment 453616 [details]
Patch
Comment 6 Kenneth Russell 2022-03-02 12:46:52 PST
Comment on attachment 453616 [details]
Patch

Looks great. r+
Comment 7 Kimmo Kinnunen 2022-03-03 02:57:54 PST
Created attachment 453714 [details]
Patch
Comment 8 Kimmo Kinnunen 2022-03-03 03:11:57 PST
Created attachment 453716 [details]
Patch for landing
Comment 9 Kimmo Kinnunen 2022-03-03 04:22:19 PST
*** Bug 236964 has been marked as a duplicate of this bug. ***
Comment 10 Darin Adler 2022-03-03 11:28:01 PST
Comment on attachment 453714 [details]
Patch

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

> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:817
> +        auto it = contexts.begin();
> +        auto* earliest = *it;
> +        for (++it; it != contexts.end(); ++it) {
> +            if (earliest->activeOrdinal() > (*it)->activeOrdinal())
> +                earliest = *it;
> +        }

I might have written this with a modern for loop even though the loop is less tight:

    WebGLRenderingContextBase* earliest = nullptr;
    for (auto& context : contexts) {
        if (!earliest || earliest->activeOrdinal() > context.activeOrdinal())
            earliest = &context;
    }

Or using an algorithm:

    auto* earliest = *std::min_element(contexts.begin(), contexts.end(), [] (auto& a, auto& b) {
        return a.activeOrdinal() > b.activeOrdinal();
    });

Or some day (not now):

    auto* earliest = std::ranges::min_element(contexts, [] (auto& a, auto& b) {
        return a.activeOrdinal() > b.activeOrdinal();
    });

I may have the lambda backwards, not sure, so please don’t just paste.
Comment 11 EWS 2022-03-03 22:04:14 PST
Committed r290816 (248052@main): <https://commits.webkit.org/248052@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 453716 [details].
Comment 12 Kimmo Kinnunen 2022-03-04 02:32:24 PST
(In reply to Darin Adler from comment #10)
> Or using an algorithm:
> 
>     auto* earliest = *std::min_element(contexts.begin(), contexts.end(), []
> (auto& a, auto& b) {
>         return a.activeOrdinal() > b.activeOrdinal();
>     });
> 


Thanks for the suggestion, bug 237464 tracks this.