Use Strict in Plutus Tx #5371

zliu41 · 2023-06-06T02:19:00Z

No description provided.

…1/strict

zliu41

Ready for review. This led to cost reduction in general, and in some cases the savings are extraordinary (e.g., from 7... to 2...). There are also some cost increases but they are much milder.

plutus-tx-plugin/test/Plugin/Coverage/Spec.hs

michaelpj

I spent a while looking through the PIR and spotted some oddities, mostly things we want to be sure that optimization is clearing up later.

There are a few things that seem actually wrong, but mostly this looks fine.

michaelpj · 2023-08-04T09:20:09Z

plutus-tx-plugin/test/Budget/andCheap.pir-readable.golden

-         (c
-            True
+let
+  !ls : List Bool


now this is strict and not obviously pure, so we can't inline it, alas.

not obviously pure

How come? Looks obviously pure to me: a lambda behind a type let.

And even if it wasn't pure, it's evaluated right after the let process, so we could still inline it. I guess teaching that the inliner might be non-trivial though. Particuarlly because the "right after" part may get changed in the course of optimization.

It's a little hard to read, but it's actually an application of the lambda inside the let. Now, if we inlined the type binding, we could then reduce the application and we would be in a better place...

plutus-tx-plugin/test/Budget/show.pir-readable.golden

plutus-tx-plugin/test/IsData/deconstructData.pir.golden

plutus-tx/plutus-tx.cabal

plutus-tx/src/PlutusTx/Builtins.hs

plutus-tx/src/PlutusTx/IsData/Class.hs

plutus-tx/src/PlutusTx/IsData/Instances.hs

effectfully

I think my biggest issue with reviewing this PR is that I don't know what to expect when I'm reviewing changes to a golden file. It would be great to have dump a comment in each golden file specifying which optimizations were performed on the code in the file, because otherwise I have no idea if something isn't, say, inlined because of an issue or because the inliner wasn't run on the program in the first place.

effectfully · 2023-08-07T16:34:11Z

plutus-benchmark/bls12-381-costs/test/bls12-381-costs.golden

+  120    1402   (8.6%)      8201088120  (82.0%)          510702   (3.6%) 
+  130    1512   (9.2%)      8884555240  (88.8%)          553062   (4.0%) 
+  140    1622   (9.9%)      9568022360  (95.7%)          595422   (4.3%) 
+  150    1732  (10.6%)     10251489480 (102.5%)          637782   (4.6%) 


Everything's slower?

Everything's slower?

Not everything, but things are a bit less slower now thanks to -fno-full-laziness!

effectfully · 2023-08-07T16:35:46Z

plutus-benchmark/bls12-381-costs/test/bls12-381-costs.golden

+  120    1402   (8.6%)     24946946570 (249.5%)          515004   (3.7%) 
+  130    1512   (9.2%)     27026011310 (270.3%)          557724   (4.0%) 
+  140    1622   (9.9%)     29105076050 (291.1%)          600444   (4.3%) 
+  150    1732  (10.6%)     31184140790 (311.8%)          643164   (4.6%) 


effectfully · 2023-08-07T16:36:05Z

plutus-benchmark/bls12-381-costs/test/bls12-381-costs.golden

+  120   12434  (75.9%)      4374611330  (43.7%)          490704   (3.5%) 
+  130   13464  (82.2%)      4739320550  (47.4%)          531424   (3.8%) 
+  140   14494  (88.5%)      5104029770  (51.0%)          572144   (4.1%) 
+  150   15524  (94.8%)      5468738990  (54.7%)          612864   (4.4%) 


And everywhere so far.

effectfully · 2023-08-07T16:36:26Z

plutus-benchmark/ed25519-costs/test/ed25519-costs.golden

+  120   21265 (129.8%)      8951320962  (89.5%)         6204546  (44.3%) 
+  130   22992 (140.3%)      9696801862  (97.0%)         6719976  (48.0%) 
+  140   24719 (150.9%)     10442282762 (104.4%)         7235406  (51.7%) 
+  150   26446 (161.4%)     11187763662 (111.9%)         7750836  (55.4%) 


effectfully · 2023-08-07T16:38:19Z

plutus-benchmark/lists/test/Sum/right-fold-built-in.budget.golden

+({cpu: 181259654
+| mem: 594532})


Still. And this is quite worse! 7.6% worse to be precise, that's quite a lot, imagine an "optimization" of the CEK machine making things so much worse.

effectfully · 2023-08-07T17:08:29Z

plutus-tx-plugin/test/Budget/sum.pir-readable.golden

@@ -6,6 +6,14 @@ in
 let
  data (AdditiveMonoid :: * -> *) a | AdditiveMonoid_match where
    CConsAdditiveMonoid : (\a -> a -> a -> a) a -> a -> AdditiveMonoid a
+  !ls : List integer
+    = (let
+          a = List integer


Or we could make the optimizer look look through type lets. I do agree it's worth inlining this type though.

effectfully · 2023-08-07T17:23:35Z

plutus-tx-plugin/test/Budget/show.uplc-readable.golden

+                                                               force
+                                                                 (force
+                                                                    (foldr
+                                                                       (delay
+                                                                          (\x ->
+                                                                             x))))


I was puzzled seeing foldr id z fs, but apparently that is the same as foldr (.) id fs z. All those times I wrote the latter and not the former...

plutus-tx-plugin/test/Plugin/Data/families/associatedParam.pir.golden

plutus-tx/src/PlutusTx/Builtins.hs

zliu41 · 2023-08-07T21:31:10Z

Many tests cases became more expensive with b267848, which stopped exposing the unfolding of plc. Previously I think the unfolding of plc prevented them from being costed properly.

Overall, there are both increased and decreased costs, but the increase is always relatively mild, while the decrease is sometimes extraordinary.

This makes sense: by making things more strict, you sometimes evaluate something once, which does not need to be evaluated. On the other hand, by making things less strict, you sometimes evaluate something many times, which only needs to be evaluated once.

…1/strict

zliu41

Keep in mind that another reason for the higher cost when using Strict comes from the fact that strict bindings are less likely to be inlined, because many of them are not considered "effect safe".

When something is not inlined, it incurs the additional machine costs of VDelay, VLamAbs and VApply, even if it doesn't change the number of times it is evaluated. But again, this is relatively mild compared to what Strict can potentially save us.

plutus-tx/src/PlutusTx/AssocMap.hs

plutus-tx-plugin/test/Plugin/Basic/ifOpt.pir.golden

michaelpj · 2023-08-08T09:24:24Z

Let me elaborate on my argument for why lazy patterns on function arguments shouldn't be able to do anything (and hence why we need to figure out WTF is actually happening).

AIUI, a bang pattern on a function argument means we get something like this (using some name shadowing because that's what we end up with also):

f !x = t
---> 
f x = case x@x of { _ -> t }
---> (PIR)
let f = \x -> let !x = x in t 
in ...
---> (inlining: the outer x is evaluated strictly, so the inliner should inline the let binding since it's a trivial binding to a strict variable)
let f = \x -> t
in ...

Presumably with Strict, a lazy pattern means that we don't do this, so we get

f x = t
---> (PIR)
let f = \x -> t

i.e. the same thing.

It cannot matter whether we inline f. We don't even need an example: inlining a function cannot make a difference to how strictly an argument is evaluated otherwise that is an unsoundness in the inliner!

So it seems to me that lazy function arguments cannot do anything... except possibly make GHC do something different.

This seems pretty important to me - we need to understand what the effect of these changes are in general.

zliu41 · 2023-08-08T12:57:50Z

i.e. the same thing.

Ok, now I remember that in Plutus Tx, let x = rhs in body and (\x -> body) rhs isn't the same thing. The former is non-strict while the latter is strict. If this is what the implementation actually does, then you are right, but I'll double check. Perhaps it isn't what it does.

michaelpj · 2023-08-08T14:34:35Z

Ok, now I remember that in Plutus Tx, let x = rhs in body and (\x -> body) rhs isn't the same thing. The former is non-strict while the latter is strict.

I don't think that matters here? The only let bindings were in the PIR-psuedocode.

zliu41

Rebased and responded to some comments.

plutus-tx-plugin/test/Budget/show.pir-readable.golden

plutus-tx/src/PlutusTx/AssocMap.hs

plutus-tx/src/PlutusTx/Builtins/Internal.hs

plutus-tx-plugin/plutus-tx-plugin.cabal

…1/strict

zliu41 · 2023-08-23T02:38:24Z

@michaelpj @effectfully One of the main mysteries solved. Previously unless I did all of these three things: link1, link2, link3, I got the strange "no unfolding" error for PlutusTx.Ratio.$fFromDataRational_$s$fFromData(,)_$cfromBuiltinData, when compiling fromBuiltinData @Rational. I was looking for the unfolding in PlutusTx.IsData.Instances, but turns out it is in PlutusTx.Ratio. With Strict on, the unfolding of this function becomes a bit larger, and exceeds GHC's threshold. This is a function created by GHC as a result of -fspecialise (as hinted by the $s), so it doesn't have the INLINABLE pragma.

The fix is simple - adding any one of the following to PlutusTx.Ratio works: -fno-specialise, or -fexpose-all-unfoldings, or bumping -funfolding-creation-threshold.

Next up: figure out why some things don't work in GHC 9.6 🤷‍♂️

…1/wip-strict

zliu41 · 2023-08-30T18:56:03Z

Added

{- Note [Lazy patterns on function parameters]
Lazy patterns (~) on function parameters don't do anything if the function isn't inlined.
This is because function applications in Plutus Tx are strict, so when passing an argument
to a function, the argument will be evaluated immediately, regardless of whether the
corresponding function parameter is lazy or strict. Specifically, both f !x = body and
f ~x = body in Plutus Tx are compiled into let f = \x -> body in PIR.

It doesn't matter whether or not the function is inlined by the PIR inliner, since the PIR
inliner does not change the semantics of the program.

However, it does make a difference if the function is unconditionally inlined by GHC.
Consider the Plutus Tx code let f !x = t in f arg. Here arg will be immediately when
being passed to f (as it should). But now consider let f ~x = t in f arg. Here GHC
may unconditionally inline f, leading to t [x := arg] (recognize that the GHC inliner
also performs beta reduction, in addition to replacing f with its definition), where arg
may not be evaluated. GHC does so because it is a Haskell compiler and does not know that
Plutus Tx is a strict language.

Therefore, lazy patterns should generally be avoided, since its semantics depend on
what the GHC inliner does. That said, we can use it in some cases to our advantage,
such as in (&&) and (||). These logical operators would be much less useful if
they can't short-circuit. By using a lazy pattern on the second parameter, we can achieve
the desired effect, namely, the second parameter is only evaluated if needed.
-}

…1/strict

zliu41

I think I've addressed all the concerns now (except doing profiling).

By the way, for some of the cost increases, their costs should have been higher before. Consider Nothing <*> _ = Nothing. The _ should always have been evaluated strictly, but, just like the || and && situation, when we didn't have Strict, GHC may unconditionally inline <*>, thereby avoiding evaluating _. This may very well be responsible for most of the cost increases, since it leads to lots of strict, dead bindings, e.g., #5371 (comment).

We can potentially do the same thing to <*> as what we did to || and &&, but since there is no dramatic cost increases (and there shouldn't be), I'd rather leave this PR as it is and do that separately.

plutus-benchmark/nofib/test/formulaBudget.budget.golden

zliu41 · 2023-08-30T23:08:48Z

plutus-tx-plugin/test/IsData/deconstructData.pir.golden

+                            (nonrec)
+                            (termbind
+                              (strict)
+                              (vardecl ds [ Maybe b ])


I think this is from Nothing <*> _ = Nothing, and it is the same situation as || and &&. The second argument should always be strictly evaluated, but when we didn't have Strict, GHC may unconditionally inline <*> which avoided evaluating the second argument.

This can be quite bad, and may be a main contributor of some of the increased budgets 😐

zliu41 · 2023-08-30T23:11:54Z

plutus-tx-plugin/test/Plugin/Strict/issue4645Eval.eval-cek-log.golden

Added:

-- TODO: the Cek log of this test case is currently incorrect as it doesn't preserve
-- the order of logging, which it should do given the conservative-optimisation flag.
-- Both GHC and ourselves are culprits in this instance, see
-- #5371 (comment)

plutus-tx/src/PlutusTx/AssocMap.hs

michaelpj · 2023-08-31T09:07:01Z

plutus-tx/src/PlutusTx/AssocMap.hs

-    , all
-    , mapThese
-    ) where
+  Map,


I'm really not a fan of the ad-hoc reformatting. It makes it much harder to review diffs. If we're going to switch to using a formatter globally, then let's agree that and apply it one go. (and if we haven't agreed and "reformatting in PRs" is acceptable, then there is nothing stopping someone just blindly formatting it back later!)

I did this to trigger CI. I should have used a different module.

We've agreed on using 2-space indentation, and our style guide has been updated, so the reformatting I did makes it conform to the style guide.

I forgot that we agreed to that :| can we revisit just agreeing to fourmolu everything?

We don't need to agree to fourmolu. Fourmolu is one way to convert 4-space to 2-space. It's not a problem for me if someone prefers hand formatting the code.

I'm not aware of any other tool that people use to auto-format code, so it's unlikely that someone will blindly re-format it in a very different format.

michaelpj · 2023-08-31T09:09:34Z

plutus-tx/src/PlutusTx/Bool.hs

+This is because function applications in Plutus Tx are strict, so when passing an argument
+to a function, the argument will be evaluated immediately, regardless of whether the
+corresponding function parameter is lazy or strict. Specifically, both `f !x = body` and
+`f ~x = body` in Plutus Tx are compiled into `let f = \x -> body` in PIR.


No, this is wrong. They are compiled differently.

f ~x = body --> f = \x -> body f !x = body --> f = \x -> let !x = x in body

That's why there is a difference if GHC inlines them, otherwise there would be literally no difference!

But aren't these two equivalent? And the second term becomes the first term after inlining. That's the point of this paragraph - it should (although it sometimes doesn't because of GHC inlining) produce the same PIR eventually, whether function parameters are strict or lazy.

Yes, they do become the same thing eventually, but the relevant thing is what they become initially, since that's what lets us observe the difference!

Okay, maybe let me phrase this differently.

f ~x = body ---> (GHC Core) f x = body ---> (PIR) f = \x -> body f !x = body ---> (GHC Core) f x = case x@x of { _ -> body } ---> (PIR) f = \x -> let x = x in body

The two PIR bindings have the same semantics. But if GHC inlines the function at the core stage, then we get the body of the second PIR binding at the call site, which does behave differently.

(I think we both agree with what's going on, I just think the explanation isn't quite clear for future people)

Ok, I added the steps you laid out above in the Note.

plutus-tx/src/PlutusTx/Bool.hs

michaelpj · 2023-08-31T09:11:45Z

plutus-tx/src/PlutusTx/Bool.hs

+Consider the Plutus Tx code `let f !x = t in f arg`. Here `arg` will be immediately when
+being passed to `f` (as it should). But now consider `let f ~x = t in f arg`. Here GHC
+may unconditionally inline `f`, leading to `t [x := arg]` (recognize that the GHC inliner
+also performs beta reduction, in addition to replacing `f` with its definition), where `arg`


It's not necessary for GHC to perform beta-reduction, although perhaps it does, because of the difference in how lazy/strict patterns get compiled above.

Yes it is! I keep forgetting this! Maybe needs a more worked out example since apparently it's not obvious lol

michaelpj · 2023-08-31T09:12:07Z

plutus-tx/src/PlutusTx/Bool.hs

+Therefore, lazy patterns should generally be avoided, since its semantics depend on
+what the GHC inliner does. That said, we can use it in some cases to our advantage,
+such as in `(&&)` and `(||)`. These logical operators would be much less useful if
+they can't short-circuit. By using a lazy pattern on the second parameter, we can achieve


Suggested change

they can't short-circuit. By using a lazy pattern on the second parameter, we can achieve

they can't short-circuit. By using a lazy pattern on the second parameter and forcing GHC to inline, we can achieve

I disagree with this. The INLINE pragma does not force GHC to inline. We run the GHC simplifier with sm_inline turned off. It does force the PIR inliner to inline, but the PIR inliner only replaces the variable with the definition, which isn't sufficient to make it non-strict. We should make it so that the INLINE pragma forces the PIR simplifier to beta-reduce. We had a length discussion in the ADR PR and IIRC this was the conclusion.

You are right yes, we need to both force inlining and beta reduction. I keep forgetting that!

We had a length discussion in the ADR PR and IIRC this was the conclusion.

I do not think we agreed to that :p #5270 (comment)

I'm overall then confused by this line: since we don't have such behaviour at the moment, we can't achieve what's written here? So the note is talking about some hypothetical future behaviour. I think we should be clear that it's not something we can do now!

Agreed. I rephrase it in an attempt to make it more clear:

Using a lazy pattern on the second parameter helps achieve
the desirable effect, namely, the second parameter is only evaluated if needed, although
this is currently not guaranteed, since (&&) and (||) are not necessarily
unconditionally inlined by GHC. To guarantee that the second parameter is only evaluated
when needed, we can potentially use a particular source annotation to tell the PIR
inliner to inline and beta-reduce a function, which would achieve the same effect as if
it is unconditionally inlined by GHC. This is yet to be implemented.

plutus-tx/src/PlutusTx/Builtins.hs

plutus-benchmark/nofib/test/formulaBudget.budget.golden

…1/strict

Co-authored-by: Michael Peyton Jones <michael.peyton-jones@iohk.io>

…o HEAD

zliu41

Great suggestion re: -fno-full-laziness. I added a Note [-fno-full-laziness in Plutus Tx].
This should now be mergeable.

zliu41 · 2023-09-01T18:04:59Z

plutus-benchmark/bls12-381-costs/test/bls12-381-costs.golden

+  120    1402   (8.6%)      8201088120  (82.0%)          510702   (3.6%) 
+  130    1512   (9.2%)      8884555240  (88.8%)          553062   (4.0%) 
+  140    1622   (9.9%)      9568022360  (95.7%)          595422   (4.3%) 
+  150    1732  (10.6%)     10251489480 (102.5%)          637782   (4.6%) 


Everything's slower?

Not everything, but things are a bit less slower now thanks to -fno-full-laziness!

zliu41 · 2023-09-01T18:08:16Z

plutus-tx/src/PlutusTx/AssocMap.hs

-    , all
-    , mapThese
-    ) where
+  Map,


We don't need to agree to fourmolu. Fourmolu is one way to convert 4-space to 2-space. It's not a problem for me if someone prefers hand formatting the code.

I'm not aware of any other tool that people use to auto-format code, so it's unlikely that someone will blindly re-format it in a very different format.

zliu41 · 2023-09-01T19:00:01Z

plutus-tx/src/PlutusTx/Bool.hs

+Therefore, lazy patterns should generally be avoided, since its semantics depend on
+what the GHC inliner does. That said, we can use it in some cases to our advantage,
+such as in `(&&)` and `(||)`. These logical operators would be much less useful if
+they can't short-circuit. By using a lazy pattern on the second parameter, we can achieve


Agreed. I rephrase it in an attempt to make it more clear:

Using a lazy pattern on the second parameter helps achieve
the desirable effect, namely, the second parameter is only evaluated if needed, although
this is currently not guaranteed, since (&&) and (||) are not necessarily
unconditionally inlined by GHC. To guarantee that the second parameter is only evaluated
when needed, we can potentially use a particular source annotation to tell the PIR
inliner to inline and beta-reduce a function, which would achieve the same effect as if
it is unconditionally inlined by GHC. This is yet to be implemented.

zliu41 · 2023-09-01T19:03:35Z

plutus-tx/src/PlutusTx/Bool.hs

+This is because function applications in Plutus Tx are strict, so when passing an argument
+to a function, the argument will be evaluated immediately, regardless of whether the
+corresponding function parameter is lazy or strict. Specifically, both `f !x = body` and
+`f ~x = body` in Plutus Tx are compiled into `let f = \x -> body` in PIR.


Ok, I added the steps you laid out above in the Note.

zliu41 marked this pull request as draft June 6, 2023 02:19

Use Strict in Plutus Tx

e2625c2

zliu41 force-pushed the zliu41/strict branch from fb0d775 to e2625c2 Compare July 29, 2023 14:06

zliu41 added 9 commits August 2, 2023 08:18

Merge branch 'master' of github.com:input-output-hk/plutus into zliu4…

8925f7e

…1/strict

Improve PlutusTx.Bool

d75d6e3

Improve PlutusTx.Builtins

f1e2870

Improve PlutusTx.AssocMap

72d0e41

Remove boolOtherFunctionSimplifiesAway

2045aad

Fix ToData Void instance

c759d2a

Fix compilation for fromBuiltinData @Rational

6ac752c

Merge branch 'master' of github.com:input-output-hk/plutus into zliu4…

549d9f6

…1/strict

Update marlowe budget tests

6bbb684

zliu41 marked this pull request as ready for review August 4, 2023 14:22

zliu41 requested review from michaelpj, effectfully and a team August 4, 2023 14:22

zliu41 commented Aug 4, 2023

View reviewed changes

plutus-tx-plugin/test/Plugin/Coverage/Spec.hs Outdated Show resolved Hide resolved

michaelpj reviewed Aug 4, 2023

View reviewed changes

zliu41 added 2 commits August 4, 2023 13:27

Readable PIR for Plugin.Strict.Spec

2126392

Do not expose unfolding for plc

b267848

zliu41 force-pushed the zliu41/strict branch from dc372e5 to b267848 Compare August 5, 2023 02:44

Update golden files

a7a5519

effectfully reviewed Aug 7, 2023

View reviewed changes

zliu41 added 2 commits August 7, 2023 14:47

Merge branch 'master' of github.com:input-output-hk/plutus into zliu4…

d4e3ada

…1/strict

Changelog

cd207df

zliu41 commented Aug 7, 2023

View reviewed changes

plutus-tx/src/PlutusTx/AssocMap.hs Outdated Show resolved Hide resolved

plutus-tx-plugin/test/Plugin/Basic/ifOpt.pir.golden Outdated Show resolved Hide resolved

Add back ghc-version-support

ddb9603

zliu41 commented Aug 17, 2023

View reviewed changes

Merge branch 'master' of github.com:input-output-hk/plutus into zliu4…

57a9c6b

…1/strict

zliu41 force-pushed the zliu41/strict branch from b6a6f1a to 57a9c6b Compare August 23, 2023 01:34

expose all unfoldings in PlutusTx.Ratio

085f240

zliu41 added 3 commits August 30, 2023 11:06

Add some pragmas to plutus-tx.cabal

4dde064

Merge branch 'master' of github.com:input-output-hk/plutus into zliu4…

a752bfe

…1/wip-strict

Note [Lazy patterns on function parameters]

ab418a3

zliu41 added 5 commits August 30, 2023 12:58

Comment on ~ in AssocMap

bacb7d7

Fix an irrelevant test

07900f1

Add a comment for issue4645

77021ae

Merge branch 'master' of github.com:input-output-hk/plutus into zliu4…

69b6c2f

…1/strict

Merge branch 'master' of github.com:input-output-hk/plutus into zliu4…

47311aa

…1/strict

zliu41 commented Aug 30, 2023

View reviewed changes

michaelpj approved these changes Aug 31, 2023

View reviewed changes

zliu41 and others added 9 commits August 31, 2023 09:34

Merge branch 'master' of github.com:input-output-hk/plutus into zliu4…

9a75ae5

…1/strict

Update plutus-tx/src/PlutusTx/Bool.hs

65dfe1a

Co-authored-by: Michael Peyton Jones <michael.peyton-jones@iohk.io>

Merge branch 'zliu41/strict' of github.com:input-output-hk/plutus int…

5eecd91

…o HEAD

Merge branch 'master' of github.com:input-output-hk/plutus into HEAD

a6207f9

Turn off full laziness!

bf4f57d

fix size test

fe21438

Update the Note

bc2bcbe

more no full laziness

508b0e4

minor change

0cc2710

zliu41 commented Sep 1, 2023

View reviewed changes

zliu41 merged commit bbc4a0d into master Sep 1, 2023
7 checks passed

zliu41 deleted the zliu41/strict branch September 1, 2023 20:02

zliu41 mentioned this pull request Sep 1, 2023

Fix longitudinal benchmark #5514

Merged

	they can't short-circuit. By using a lazy pattern on the second parameter, we can achieve
	they can't short-circuit. By using a lazy pattern on the second parameter and forcing GHC to inline, we can achieve

Use Strict in Plutus Tx #5371

Use Strict in Plutus Tx #5371

Conversation

zliu41 commented Jun 6, 2023

zliu41 left a comment • edited Loading

Choose a reason for hiding this comment

michaelpj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

effectfully left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zliu41 commented Aug 7, 2023

zliu41 left a comment • edited Loading

Choose a reason for hiding this comment

michaelpj commented Aug 8, 2023

zliu41 commented Aug 8, 2023

michaelpj commented Aug 8, 2023

zliu41 left a comment

Choose a reason for hiding this comment

zliu41 commented Aug 23, 2023

zliu41 commented Aug 30, 2023

zliu41 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zliu41 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zliu41 left a comment •

edited

Loading

zliu41 left a comment •

edited

Loading