This reverts commit a0c82f08df.
Change-Id: Ic2e93591c64992ec22e477bd0975d71954bef1c6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321469
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This will later allow us to rescan parent statements independently,
instead of performing a full rescan of the entire Program on each
iteration.
Change-Id: Id86e139d81125bc529aba9453cba5606d1041908
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321462
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 3e40ce0937.
Reason for revert: Lots of bad Vk images from Pixel2/3
Original change's description:
> Convert sksl_frag.sksl to an IRIntrinsicMap
>
> Did some related cleanup:
> - We were setting the IRGenerator's fIntrinsics to the (empty) GPU map
> while converting the GPU/frag/vert source. Make the IR generator
> support a null intrinsic map, so we can (more correctly) structure the
> compiler's constructor.
> - Use explicit types on all calls to findAndInclude.
> - Move the assert back into grab_intrinsics - every converted include
> only contains supported elements (and will continue to do so).
>
> Change-Id: I80ebb247107dde656946858bf2cd1f50a03f67d3
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321496
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I6f11366971bfd252f11d30c48e2e776d30458933
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321683
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Did some related cleanup:
- We were setting the IRGenerator's fIntrinsics to the (empty) GPU map
while converting the GPU/frag/vert source. Make the IR generator
support a null intrinsic map, so we can (more correctly) structure the
compiler's constructor.
- Use explicit types on all calls to findAndInclude.
- Move the assert back into grab_intrinsics - every converted include
only contains supported elements (and will continue to do so).
Change-Id: I80ebb247107dde656946858bf2cd1f50a03f67d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321496
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
As a first step, convert sksl_pipeline.sksl to an IntrinsicMap (rather
than inherited element list). This makes the new code operate on
sk_FragCoord (which was previously being shared by all runtime effect
programs).
The new unit test angered TSAN, and now runs without complaint.
Also finish converting the .fp intrinsics over, so those don't need an
inherited element list either. And while doing that, refactor that
parsing to match all of the others. FP was uniquely implementing
processIncludeFile itself, rather than reusing the pattern of other
pre-include parsing.
The meat of the CL is the subtle changes in Compiler, and the logic in
cloneBuiltinVariables. Note that we need to clone the global variable
declaration element (because one of the goals is to get rid of shared
and inherited program elements), but also the variable itself (and the
new copy needs to live in the program's symbol table).
Bug: skia:10589
Bug: skia:10679
Bug: skia:10680
Change-Id: Ied352f8434dac2b8eacb4e515b014b6af7b57d20
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319023
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
We don't use the entrance ID values anywhere; we only check to see if
the block is reachable or not.
Change-Id: I0988e6c1999936de25dd04404409b736d8ad14f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321540
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
The inliner generally only cares whether a function has 50 nodes or
less. Once we hit the 50th node, we can stop counting.
Change-Id: I92918989a9b3b5b73c1d1f13fa25c9adfa5b7e40
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321198
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: Ib97cbf2e20580d3b9e54a5ffe793013ec97d4892
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321539
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit 135e237656.
No-Tree-Checks: true
Change-Id: I7e9dd2148f7b2a8dee1e49a9a9cc593e0d7ceb6e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321460
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Similar to the same field on Enum and FunctionDeclaration, will be used
to facilitate cloning builtin variables into Programs that use them.
Bug: skia:10589
Change-Id: Ic63701c61ee4658a5ec72adb506cc96aa0b2836f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321196
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: If2b1e8f89730bfff9e08c1ff5f5cb02c16088d86
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321117
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
We don't need to create a temporary variable for expressions like
`half3(x)`.
Change-Id: Ie0fa6a6dfb3d77d4372f96c676d3081f7e278852
Bug: skia:10786
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320960
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This allows us to traverse a program's hierarchy and make changes (as
long as the structure remains intact). It's the caller's responsibility
to make sure they don't invalidate any iterators of the ProgramWriter.
Change-Id: Icfc651134d916e19b92004c92fe09880bb96600b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320717
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit dd33b3ea90.
Change-Id: I348b2b5976966a7451d88bd7f96ce17ce1702b79
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320826
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
For instance, `foo[0].x` is now considered trivial to inline. It
combines two trivial cases: array-indexing by an int literal, and a
swizzle.
Change-Id: Ibb3ca1f324bbee0e9b3556e66644923fc9e0cf45
Bug: skia:10786
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320768
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 6877f0bfdc.
Reason for revert:
../../src/sksl/SkSLDehydrator.cpp:423:31: error: no member named 'fExpression' in 'SkSL::ExpressionStatement'
this->write(e.fExpression.get());
Original change's description:
> moved SkSL ExpressionStatement's data into IRNode
>
> Change-Id: I11b1662cd58b01fabba75dbbee40267a62c8b420
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320639
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: Id22b8a9c93e842b2775e11f5d4c173e25860b5d0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320824
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The following types of expression are hoisted directly into the
inlined code:
- Struct field access: `myStruct.myField`
- Swizzles: `myVector.xzy`
- Simple array indexes: `myArray[0]`
This significantly reduces the number of temporary variables generated
by the inliner.
Change-Id: Ifed226ecc87b096ec1e38752c0c38ae32bd31578
Bug: skia:10737, skia:10786
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319919
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit ff22910286.
Change-Id: I86619819aae169a2cb8d59ad7ccecf26423f2aa9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320764
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
This reverts commit 98503f1c57.
Reason for revert:
../../src/sksl/SkSLDehydrator.cpp:513:28: error: no member named 'fTypeName' in 'SkSL::Enum'
this->write(en.fTypeName);
~~ ^
../../src/sksl/SkSLDehydrator.cpp:514:56: error: no member named 'fSymbols' in 'SkSL::Enum'
AutoDehydratorSymbolTable symbols(this, en.fSymbols);
~~ ^
../../src/sksl/SkSLDehydrator.cpp:515:62: error: no member named 'fSymbols' in 'SkSL::Enum'
for (const std::unique_ptr<const Symbol>& s : en.fSymbols->fOwnedSymbols) {
~~ ^
Original change's description:
> moved SkSL Enum data into IRNode
>
> Change-Id: I0de52d252715b5f4e10c26ebca3ea1a4f728ea2e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320637
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I2b78dd5acf4277765b36776a8fb8e435f8b18861
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320759
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: I0de52d252715b5f4e10c26ebca3ea1a4f728ea2e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320637
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:10785
Change-Id: I01708af63d7e2ffc160022074ea9ff2b3c69eab5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320638
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I9568deca0031d32bc1c6bdf1f11f6da76de6d07f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320075
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The node's kind can be easily inferred by presence of a statement or
pointer inside of it. When there are only two kinds, having a separate
field doesn't add value. (If we end up wanting more block types in the
future, we could re-add fKind as a private field.)
Change-Id: I8e9db122b4a82728d987c4913a7bdff85b4b1a2d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320298
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This allows dead-stripping to properly optimize away unreferenced clones
of intrinsic functions, and allows the inliner to detect intrinsic
functions that are only called once (which can generally always be
inlined without penalty).
Change-Id: I0cf034d880ae5d52f4cc0f93de6e2c7aad34e975
Bug: skia:10776
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320258
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is a reland of 435b482638
inlineStatement now takes a `const Expression* resultExpr` instead of
`const Expression& resultExpr` because resultExpr will be null for a
void function.
Original change's description:
> Support out parameters that use a swizzle.
>
> This CL also removes the `VariableExpression` class that was briefly
> added in a prior CL. This class was intended to support cloning an
> expression while changing the refKind of a VariableReference inside of
> the expression, but it added state and complexity. In this CL, rather
> than track this via extra state, the inliner just recurses into the
> expression as needed to find its VariableReference. Since most relevant
> expressions are just a VariableReference anyway, this is inexpensive.
>
> Change-Id: Id4d926b7d7520b5e6ce455446c05a6d59ef62a84
> Bug: skia:10756
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319917
> Commit-Queue: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:10756
Change-Id: I35f76c21eccf0ba2ab47e4313e131f7aa26980fa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320223
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 435b482638.
Reason for revert: ASAN/UBSAN unhappy.
Original change's description:
> Support out parameters that use a swizzle.
>
> This CL also removes the `VariableExpression` class that was briefly
> added in a prior CL. This class was intended to support cloning an
> expression while changing the refKind of a VariableReference inside of
> the expression, but it added state and complexity. In this CL, rather
> than track this via extra state, the inliner just recurses into the
> expression as needed to find its VariableReference. Since most relevant
> expressions are just a VariableReference anyway, this is inexpensive.
>
> Change-Id: Id4d926b7d7520b5e6ce455446c05a6d59ef62a84
> Bug: skia:10756
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319917
> Commit-Queue: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: Ibdda47607f9e6e7f3a7459915067cf5e20919993
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320220
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Now that ternaries are no longer supported for assignment (as per GLSL
spec), there's no longer any cases where an assignment can target more
than one variable reference at a time. Replace the output vector of
VariableReferences `assignedVars` with a single VariableReference,
`assignedVar`.
Also, allow callers to pass null for the ErrorReporter. This is useful
if inability to assign to an expression does not actually indicate an
error condition.
Change-Id: I146a9d1a488131ac5048c665e4dc880d895a275a
Bug: skia:10767
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319859
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This CL also removes the `VariableExpression` class that was briefly
added in a prior CL. This class was intended to support cloning an
expression while changing the refKind of a VariableReference inside of
the expression, but it added state and complexity. In this CL, rather
than track this via extra state, the inliner just recurses into the
expression as needed to find its VariableReference. Since most relevant
expressions are just a VariableReference anyway, this is inexpensive.
Change-Id: Id4d926b7d7520b5e6ce455446c05a6d59ef62a84
Bug: skia:10756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319917
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit a05d27b170.
Reason for revert: Google3 roll,
third_party/skia/HEAD/src/sksl/SkSLTestRehydrator.cpp:311:15: error: no type named 'Dehydrator' in namespace 'SkSL'
SkSL::Dehydrator dehydrator;
Original change's description:
> moved SkSL BoolLiteral data into IRNode
>
> Change-Id: I177b6daf4d6cb024ba20264ab01d0aa68e768a6d
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319782
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I76bcdc7ef914448b439df81cd382066980e1251e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320017
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
GLSL does not support assigning to ternaries, and will fail to compile
and/or generate non-functional shaders if we pass in a shader that tries
to assign into a ternary expression.
If SkSL is able to completely eliminate the ternary (e.g. if it boils
down to a simple `true ? x : y` or `false ? x : y`), SkSL can strip out
the ternary entirely and generate valid GLSL. This case is harmless and
so it is still allowed.
Change-Id: I960f119fb9934f998697634e6c4e519cd77d3780
Bug: skia:10767
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319679
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I452e52a87d89cefb5c21a0d9d57e9771f3038d73
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319783
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This was unused and did not work on non-GLSL backends.
Change-Id: I6bd314d43cfefa64871b5c0e964b5ae52e494164
Bug: skia:10757
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319778
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
This will allow the inliner to use IsAssignable.
Change-Id: Ic94f71002779b53d0b3dc97f37fbe4bb98b026d8
Bug: skia:10756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319414
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We had lots of checks just checking defined but we always define
GR_TEST_UTILS
Change-Id: I588c50ddd91f71618a96ab6c9eda2050b423f611
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319682
Commit-Queue: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
The absence of the FieldAccess::fBase traversal appears to be a simple
oversight. This doesn't appear to affect any tests.
Change-Id: I82a5828acedd00f62bf177bd2cf70d67071a83fa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319413
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We will soon be re-targeting variable references, and this is going to
be much easier (and cheaper) than replacing the entire VariableReference
itself.
Change-Id: I8febc44a1c06e99251153f038a4f5f693cd30231
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319344
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I93ff7e5f1062c6a85152c587fcedc34e9257dd27
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319345
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I5b4fe40847112a11d6057ee7acd208879a71722f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319190
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit b61c3a9a01.
Change-Id: I42d93bdc6455c8ef941a6cbe1339df2ba916bb3c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318697
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
We don't want to be polluting the global namespace with external values,
especially when the typical/recommended way to use the Compiler is with
a single long-lived instance. Force client code to manage ownership (the
only non-unit-test case was already doing this), and pass external
values to convertProgram, so they can be added to the Program's symbol
table.
Change-Id: If4c1db5e48a62e2cf4333b8d80420f2dfede27ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319125
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
