Subject: [quicwg/base-drafts] Fixing recovery pseudocode (#3909)

From: Kazu Yamamoto <[email-masked]>
Date: Tue, 14 Jul 2020 17:16:41 -0700

- Fixing trivial syntax errors.
- Adding comments for unused variables or defined-later.
- 6d43abd changes `return` to `continue` to make sense. (need to check by reviewers)
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/3909

-- Commit Summary --

  * now -&gt; now()
  * using &quot;.sent_bytes&quot; instead of undefined &quot;.size&quot;
  * using &quot;continue&quot; instead of &quot;return&quot;.
  * using bytes_in_flight instead of words.
  * adding a comment since bytes_in_flight is not defined here yet.
  * adding comment.

-- File Changes --

    M draft-ietf-quic-recovery.md (24)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/3909.patch
https://github.com/quicwg/base-drafts/pull/3909.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909

---

From: Junho Choi <[email-masked]>
Date: Tue, 14 Jul 2020 18:13:36 -0700

@junhochoi commented on this pull request.

> @@ -1274,7 +1277,8 @@ GetPtoTimeAndSpace():
   duration = (smoothed_rtt + max(4 * rttvar, kGranularity))
       * (2 ^ pto_count)
   // Arm PTO from now when there are no inflight packets.
-  if (no in-flight packets):
+  // bytes_in_flight is defined in the next appendix.
+  if (bytes_in_flight <= 0):

How can bytes_in_flight be negative?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#pullrequestreview-448564957

---

From: Junho Choi <[email-masked]>
Date: Tue, 14 Jul 2020 18:16:30 -0700

>         if (InCongestionRecovery(packet.time_sent)):
          // Do not increase congestion window in recovery period.
-         return
+         continue

I believe these 3 `return` are correct, not `continue`. L1523 says "Do not increase congestion window in recovery period."

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#pullrequestreview-448565844

---

From: Junho Choi <[email-masked]>
Date: Tue, 14 Jul 2020 18:16:47 -0700

>         if (IsAppOrFlowControlLimited()):
          // Do not increase congestion_window if application
          // limited or flow control limited.
-         return
+         continue

Same to above.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#pullrequestreview-448565931

---

From: Junho Choi <[email-masked]>
Date: Tue, 14 Jul 2020 18:17:11 -0700

>         if (congestion_window < ssthresh):
          // Slow start.
-         congestion_window += packet.size
-         return
+         congestion_window += packet.sent_bytes
+         continue

Might be `else`, but not `continue`.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#pullrequestreview-448566069

---

From: Kazu Yamamoto <[email-masked]>
Date: Tue, 14 Jul 2020 18:25:13 -0700

@kazu-yamamoto commented on this pull request.

`bytes_in_flight` is non negative. But this coding style follows the traditional practice. ;)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r454735461

---

From: Kazu Yamamoto <[email-masked]>
Date: Tue, 14 Jul 2020 18:27:54 -0700

It is guaranteed that all acked packets satisfy `InCongestionRecovery`? If so, this first `return` is OK.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r454736149

---

From: Kazu Yamamoto <[email-masked]>
Date: Tue, 14 Jul 2020 18:30:08 -0700

To avoid confusion, `return` should not be used in `for`. `return` should be used before `for` by checking a specific packet.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r454736754

---

From: Kazu Yamamoto <[email-masked]>
Date: Tue, 14 Jul 2020 18:31:44 -0700

// Slow start.

But this should not be `return`! I like `else`.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r454737262

---

From: Kazu Yamamoto <[email-masked]>
Date: Tue, 14 Jul 2020 18:34:49 -0700

`bytes_in_flight -= packet.sent_bytes` should be evaluated for every packet, right? If so, `return` is simply wrong.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r454738023

---

From: Kazu Yamamoto <[email-masked]>
Date: Tue, 14 Jul 2020 18:37:34 -0700

The following code makes sense to me at this moment:

```
   OnPacketsAcked(acked_packets):
     for (packet in acked_packets):
       // Remove from bytes_in_flight.
       bytes_in_flight -= packet.sent_bytes

     // Do not increase congestion_window if application
     // limited or flow control limited.
     if (IsAppOrFlowControlLimited())
       return

     for (packet in acked_packets):
       if (InCongestionRecovery(packet.time_sent)):
         // Do not increase congestion window in recovery period.
         continue
       if (congestion_window < ssthresh):
         // Slow start.
         congestion_window += packet.sent_bytes
       else:
         // Congestion avoidance.
         congestion_window += max_datagram_size * acked_packet.sent_bytes
           / congestion_window
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#issuecomment-658495157

---

From: Junho Choi <[email-masked]>
Date: Tue, 14 Jul 2020 22:26:48 -0700

>          congestion_window += max_datagram_size * acked_packet.sent_bytes

Oh I see. When I implemented congestion control I used older version of the code.
I think we can simplify as follows:

```
        OnPacketAcked(packet)

OnPacketAcked(packet):

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#issuecomment-658553570

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 00:06:16 -0700

Yeah. Your code is faster than mine since yours traverses `acked_packets` only once.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#issuecomment-658587125

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 00:15:21 -0700

I have rebased this PR to fix conflict, dropped the commit of `continue` and add a commit suggested by @junhochoi. (The syntax of `for` is fixed. :-)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#issuecomment-658590942

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 00:32:16 -0700

@kazu-yamamoto pushed 2 commits.

0aa7b82606ac4d1fbe2fa43aa7d9e709d6f48110  fixing OnPacketsAcked
497cd8fafaad3e18564e7c01a0b4f1c597c9cbe3  defining IncludesAckEliciting.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909/files/5ae745d463b4e53bf194c27a51952ba1ba8d302c..497cd8fafaad3e18564e7c01a0b4f1c597c9cbe3

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 00:34:25 -0700

Instead of adding a comment to `.ack_eliciting`, which is currently unused, I implemented `IncludesAckEliciting` which uses `.ack_eliciting`.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#issuecomment-658599272

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 00:38:16 -0700

@kazu-yamamoto pushed 1 commit.

5e99c578302b8b5421acca273da11eb29a56481f  unifying packet and acked_packet since acked_packet was not defined anywhere.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909/files/497cd8fafaad3e18564e7c01a0b4f1c597c9cbe3..5e99c578302b8b5421acca273da11eb29a56481f

---

From: ianswett <[email-masked]>
Date: Wed, 15 Jul 2020 14:46:51 -0700

@ianswett commented on this pull request.

Thanks for the fixes, but let's hold off on referencing bytes_in_flight in the recovery pseudocode for now.

bytes_in_flight isn't introduced until the congestion control section, which is why I didn't do this.  We could merge the pseudocode from both, but I believe doing that is a larger project than this PR, so I don't want to do this in this PR.

> -         return
-       // Congestion avoidance.
-       congestion_window += max_datagram_size * acked_packet.size
-           / congestion_window
+  OnPacketsAcked(acked_packets):
+    for acked_packet in acked_packets:
+      OnPacketAcked(acked_packet)
+
+  OnPacketAcked(acked_packet):
+    // Remove from bytes_in_flight.
+    bytes_in_flight -= acked_packet.sent_bytes
+
+    // Do not increase congestion_window if application
+    // limited or flow control limited.
+    if (IsAppOrFlowControlLimited())
+        return

Please change back to 2-space indent

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#pullrequestreview-449365569

---

From: Junho Choi <[email-masked]>
Date: Wed, 15 Jul 2020 14:51:46 -0700

```suggestion
      return
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#pullrequestreview-449369663

---

From: Junho Choi <[email-masked]>
Date: Wed, 15 Jul 2020 14:51:58 -0700

+    // Do not increase congestion window in recovery period.
+    if (InCongestionRecovery(acked_packet.time_sent)):
+        return

```suggestion
      return
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#pullrequestreview-449369759

---

From: Junho Choi <[email-masked]>
Date: Wed, 15 Jul 2020 14:52:23 -0700

> +        // Slow start.
+        congestion_window += acked_packet.sent_bytes

```suggestion
      // Slow start.
      congestion_window += acked_packet.sent_bytes
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#pullrequestreview-449369981

---

From: Junho Choi <[email-masked]>
Date: Wed, 15 Jul 2020 14:53:14 -0700

> +        // Congestion avoidance.
+        congestion_window += max_datagram_size * acked_packet.sent_bytes
+            / congestion_window

```suggestion
      // Congestion avoidance.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#pullrequestreview-449370406

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 17:41:27 -0700

9e2a62d4a8c6a07b839e76db63ad5fb9fd52be67  fixing indentation in pseudocode

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909/files/5e99c578302b8b5421acca273da11eb29a56481f..9e2a62d4a8c6a07b839e76db63ad5fb9fd52be67

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 17:42:16 -0700

89dcd5606fbd022a7a8e725381eefb6582fbdbb3  fixing indentation in pseudocode

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909/files/9e2a62d4a8c6a07b839e76db63ad5fb9fd52be67..89dcd5606fbd022a7a8e725381eefb6582fbdbb3

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 17:42:37 -0700

7cb8045ea2edd03b78a2aa1bad9b185a6f2c6779  fixing indentation in pseudocode

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909/files/89dcd5606fbd022a7a8e725381eefb6582fbdbb3..7cb8045ea2edd03b78a2aa1bad9b185a6f2c6779

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 17:42:51 -0700

34db480bd5fe5775668edee7ee9cf8416fface95  fixing indentation in pseudocode

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909/files/7cb8045ea2edd03b78a2aa1bad9b185a6f2c6779..34db480bd5fe5775668edee7ee9cf8416fface95

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 17:49:05 -0700

c11b0dd7a8f40d33c6648aca8e3077b0be5c1653  fixing indentation of pseudocode.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909/files/34db480bd5fe5775668edee7ee9cf8416fface95..c11b0dd7a8f40d33c6648aca8e3077b0be5c1653

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 17:50:01 -0700

> Please change back to 2-space indent

@ianswett This has been fixed by @junhochoi. Thanks!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r455446102

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 17:52:09 -0700

@ianswett I found that the left margins of pseudo code are also inconsistent. Some have 3 spaces, some have 2 spaces and the others have nothing. Would you suggest which one is correct?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#issuecomment-659090382

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 17:57:59 -0700

> Thanks for the fixes, but let's hold off on referencing bytes_in_flight in the recovery pseudocode for now.

I suggested this because `bytes_in_flight` is also used in A.9. Should I change both `bytes_in_flight` in Appendix A to "no in-flight packets"?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#issuecomment-659091929

---

From: ianswett <[email-masked]>
Date: Wed, 15 Jul 2020 19:11:32 -0700

I believe 2 space indent is correct, and 3 is an inadvertent error.

For the bytes_in_flight change, I'd prefer you revert that for now and we can decide separately if we want to merge the congestion control and loss recovery pseudocode into one section with a single set of variables.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#issuecomment-659114186

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 21:10:48 -0700

@kazu-yamamoto pushed 4 commits.

07e87d3dce371b016b69c47ade45b9bb7858820e  fixing OnPacketsAcked
1a689213f8bd66f411f6ab56d18711979c0e40e4  defining IncludesAckEliciting.
65dc7ea163cffb7bb5645973b121ebe12b100678  unifying packet and acked_packet since acked_packet was not defined anywhere.
f68b55397402a49b753c5e2cb3d5e52e05a1262e  fixing indentation in pseudocode

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909/files/c11b0dd7a8f40d33c6648aca8e3077b0be5c1653..f68b55397402a49b753c5e2cb3d5e52e05a1262e

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 21:11:58 -0700

@ianswett I dropped two commits about `bytes_in_flight` and squashed commits relating to spaces.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#issuecomment-659146575

---

From: Martin Thomson <[email-masked]>
Date: Wed, 15 Jul 2020 21:28:15 -0700

@martinthomson approved this pull request.

As long as we are comfortable with the change in behaviour, this is ok.  I think that this is more correct than the previous, but that's just me.

> @@ -1154,7 +1154,7 @@ Pseudocode for OnPacketSent follows:
      if (ack_eliciting):
        time_of_last_ack_eliciting_packet[pn_space] = now()
      OnPacketSentCC(sent_bytes)
-     sent_packets[pn_space][packet_number].size = sent_bytes
+     sent_packets[pn_space][packet_number].sent_bytes = sent_bytes

```suggestion
     sent_packets[pn_space][packet_number].sent_bytes =
       sent_bytes
```

+        / congestion_window

```suggestion
      congestion_window += 
        max_datagram_size * acked_packet.sent_bytes
        / congestion_window
```

Not quite sure to do about this one.  I would normally add a temporary variable in cases like this, but this probably isn't the best place for that.

An alternative factoring is:

```python
    incr = acked_packet.sent_bytes
    if (congestion_window >= ssthresh):
      incr = incr * max_datagram_size / congestion_window
    congestion_window += incr
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#pullrequestreview-449496276

---

From: Marten Seemann <[email-masked]>
Date: Wed, 15 Jul 2020 22:20:45 -0700

@martinthomson Where's the behavioral change here?
I find this PR a bit hard to review, since it bunches together unrelated changes.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#issuecomment-659165208

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 22:23:33 -0700

+        / congestion_window

@martinthomson My implementation is something like yours.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r455516856

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 22:30:43 -0700

@marten-seemann The behavior of `OnPacketsAcked` is changed. `return` in the original code is not correct. In some cases, it returns even if there are more acked packets. Therefore ` bytes_in_flight` is not updated correctly. This PR introduced `OnPacketAcked` so that `return` makes sense.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#issuecomment-659168318

---

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 22:33:06 -0700

if (ack_eliciting):

@martinthomson Do you mean that this line is too long?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r455519595

---

From: Martin Thomson <[email-masked]>
Date: Wed, 15 Jul 2020 22:56:51 -0700

@martinthomson commented on this pull request.

      if (ack_eliciting):

Yes.  That is the error that CI is reporting.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r455526581

---

From: Martin Thomson <[email-masked]>
Date: Wed, 15 Jul 2020 22:57:22 -0700

+        / congestion_window

As I noted above, you might want to use this instead to deal with the CI issues.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r455526756

---

From: Kazu Yamamoto <[email-masked]>
Date: Thu, 16 Jul 2020 00:07:14 -0700

652aa2e1959c77875000cf72386391ad67d9947d  Wrapping a line of pseudocode to fix CI

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909/files/f68b55397402a49b753c5e2cb3d5e52e05a1262e..652aa2e1959c77875000cf72386391ad67d9947d

---

From: Kazu Yamamoto <[email-masked]>
Date: Thu, 16 Jul 2020 00:08:00 -0700

if (ack_eliciting):

I understand. Thanks!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r455555919

---

From: Kazu Yamamoto <[email-masked]>
Date: Thu, 16 Jul 2020 00:21:34 -0700

8a4861f8d9dbe9ef785173f24ae11c18facd8039  Wrapping a line of pseudocode to fix CI

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909/files/652aa2e1959c77875000cf72386391ad67d9947d..8a4861f8d9dbe9ef785173f24ae11c18facd8039

---

From: Kazu Yamamoto <[email-masked]>
Date: Thu, 16 Jul 2020 00:22:22 -0700

+        / congestion_window

I took the wrapping one. Thanks.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r455563105

---

From: Junho Choi <[email-masked]>
Date: Thu, 16 Jul 2020 00:36:16 -0700

+    // limited or flow control limited.
+    if (IsAppOrFlowControlLimited())
+      return
+
+    // Do not increase congestion window in recovery period.
+    if (InCongestionRecovery(acked_packet.time_sent)):
+      return
+
+    if (congestion_window < ssthresh):
+      // Slow start.
+      congestion_window += acked_packet.sent_bytes
+    else:
+      // Congestion avoidance.
+      congestion_window += 
+        max_datagram_size * acked_packet.sent_bytes

While we are talking about this... this formula may not increase cwnd at all when acked_packet.sent_bytes is pretty small and congestion_window is high (meaning, when mss x sent_bytes < cwnd), because this is integer operation in most of the implementation. It may lead to too conversative cwnd growth during congestion avoidance.

One solution is to use RFC3465 (TCP ABC) style congestion avoidance. https://www.rfc-editor.org/rfc/rfc3465.html#section-2.1

If you guys think it's beneficial I'd open a new PR.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#pullrequestreview-449586215

---

From: Kazu Yamamoto <[email-masked]>
Date: Thu, 16 Jul 2020 01:10:47 -0700

c2ffa5a4b025cb221eeec0865fd460354213a860  removing trailing space.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909/files/8a4861f8d9dbe9ef785173f24ae11c18facd8039..c2ffa5a4b025cb221eeec0865fd460354213a860

---

From: Kazu Yamamoto <[email-masked]>
Date: Thu, 16 Jul 2020 01:16:20 -0700

Let's fix it in another PR.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r455606349

---

From: Kazu Yamamoto <[email-masked]>
Date: Thu, 16 Jul 2020 01:16:37 -0700

CI passed.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#issuecomment-659239097

---

From: ianswett <[email-masked]>
Date: Thu, 16 Jul 2020 06:55:10 -0700

Yes, using integer math can be a problem here, but I expect each implementation to do what make sense for them to avoid rounding issues.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#discussion_r455803887

---

From: ianswett <[email-masked]>
Date: Thu, 16 Jul 2020 06:56:01 -0700

@ianswett approved this pull request.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#pullrequestreview-449871834

---

From: ianswett <[email-masked]>
Date: Thu, 16 Jul 2020 06:56:10 -0700

Merged #3909 into master.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3909#event-3553347178