Issue 10945 in skia: SkSL: inliner will OOM in ExponentialGrowth.sksl if fInlineThreshold is set extremely high

[johns… via monorail]

Status: Accepted
Owner: johns...@google.com
Area: SkSL
Priority: Medium
Type: Defect

New issue 10945 by johns...@google.com: SkSL: inliner will OOM in ExponentialGrowth.sksl if fInlineThreshold is set extremely high
https://bugs.chromium.org/p/skia/issues/detail?id=10945

With fInlineThreshold set to 10000, the test case "tests/sksl/inliner/ExponentialGrowth.sksl" will cause skslc to consume 120GB of memory and fail with OOM.

Inlining can be very helpful within reason, but we need to enforce a hard cap and stop inlining eventually; eventually the cost can outweigh the benefit.

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

[johns… via monorail]

Updates:
Status: Fixed

Comment #1 on issue 10945 by johns...@google.com: SkSL: inliner will OOM in ExponentialGrowth.sksl if fInlineThreshold is set extremely high
https://bugs.chromium.org/p/skia/issues/detail?id=10945#c1

(No comment was entered for this change.)

[bugdroid via monorail]

Comment #2 on issue 10945 by bugdroid: SkSL: inliner will OOM in ExponentialGrowth.sksl if fInlineThreshold is set extremely high
https://bugs.chromium.org/p/skia/issues/detail?id=10945#c2

The following revision refers to this bug:
https://skia.googlesource.com/skia/+/053739dfa86f6bc01188e531e8e9e68ab13e893d

commit 053739dfa86f6bc01188e531e8e9e68ab13e893d
Author: John Stiles <johns...@google.com>
Date: Fri Nov 13 22:13:21 2020

Add unit test for O(n^3) behavior in the inliner.

In practice, the inline threshold does a good job of limiting the
blast radius here.

Change-Id: I495184116e733262ea9d84fec30885ea047ca116
Bug: skia:10945
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334597
Commit-Queue: John Stiles <johns...@google.com>
Auto-Submit: John Stiles <johns...@google.com>
Reviewed-by: Brian Osman <brian...@google.com>

[modify] https://crrev.com/053739dfa86f6bc01188e531e8e9e68ab13e893d/gn/sksl_tests.gni
[add] https://crrev.com/053739dfa86f6bc01188e531e8e9e68ab13e893d/tests/sksl/inliner/golden/ExponentialGrowth.glsl
[add] https://crrev.com/053739dfa86f6bc01188e531e8e9e68ab13e893d/tests/sksl/inliner/ExponentialGrowth.sksl

[bugdroid via monorail]

Comment #3 on issue 10945 by bugdroid: SkSL: inliner will OOM in ExponentialGrowth.sksl if fInlineThreshold is set extremely high
https://bugs.chromium.org/p/skia/issues/detail?id=10945#c3

The following revision refers to this bug:

https://skia.googlesource.com/skia/+/031a76756e245fdb862a5d17b8ed57d5b4d94ad2

commit 031a76756e245fdb862a5d17b8ed57d5b4d94ad2
Author: John Stiles <johns...@google.com>
Date: Fri Nov 13 23:02:11 2020

Stop the inliner after it has inlined 2500 statements in a program.

This prevents OOMing when given a pathological input, but is large
enough that almost all inputs should continue to compile as-is.

Change-Id: If5c46711b886ee08495bfd09af537e9dc7ea5649
Bug: skia:10945, oss-fuzz:27442
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334838
Commit-Queue: John Stiles <johns...@google.com>
Reviewed-by: Brian Osman <brian...@google.com>
Auto-Submit: John Stiles <johns...@google.com>

[modify] https://crrev.com/031a76756e245fdb862a5d17b8ed57d5b4d94ad2/src/sksl/SkSLMain.cpp
[add] https://crrev.com/031a76756e245fdb862a5d17b8ed57d5b4d94ad2/tests/sksl/inliner/golden/ExponentialGrowthStandaloneSettings.glsl
[modify] https://crrev.com/031a76756e245fdb862a5d17b8ed57d5b4d94ad2/gn/sksl_tests.gni
[modify] https://crrev.com/031a76756e245fdb862a5d17b8ed57d5b4d94ad2/tests/sksl/inliner/ExponentialGrowth.sksl
[modify] https://crrev.com/031a76756e245fdb862a5d17b8ed57d5b4d94ad2/src/sksl/SkSLInliner.h
[modify] https://crrev.com/031a76756e245fdb862a5d17b8ed57d5b4d94ad2/src/sksl/SkSLInliner.cpp
[modify] https://crrev.com/031a76756e245fdb862a5d17b8ed57d5b4d94ad2/tests/sksl/inliner/golden/ExponentialGrowth.glsl