Scheduling #1328
Scheduling #1328
Conversation
| It's important to call `wait` at the end of the `run` loop if there is nothing | ||
| left to do. Otherwise that thread will tend to use up a lot of the CPU, | ||
| plus constantly be acquiring and releasing `condition`. This busywork not only | ||
| slows down the computer, but also causes the callbacks from the `Timer` to | ||
| happen at erratic times, because the two threads are competing for the | ||
| lock.[^try-it] | ||
|
|
||
| [condition-variable]: https://docs.python.org/3/library/threading.html#threading.Condition | ||
|
|
||
| [lock-class]: https://docs.python.org/3/library/threading.html#threading.Lock | ||
|
|
||
| [^try-it]: Try removing this code and observe. The timers will become quite | ||
| erratic. | ||
|
|
There was a problem hiding this comment.
I found this confusing when reading; it makes much more sense after the following code block, so I moved it.
| with the browser's main work until the request is done.^[In theory two | ||
| parallel requests could race while accessing the cookie jar; I'm not | ||
| fixing this out of expediency but a proper implementation would have | ||
| locks for the cookie jar.] |
There was a problem hiding this comment.
I think this is the best we can really do at the moment. Hash operations might be atomic in Python but I didn't feel confident enough in this fact to put it in the footnote.
There was a problem hiding this comment.
not critical to fix this in the printed book.
| `XHR_REQUESTS` and calls its `onload` function, which is the standard | ||
| callback for asynchronous `XMLHttpRequest`s: |
There was a problem hiding this comment.
Otherwise we never mention what onload's role is.
There was a problem hiding this comment.
What do you mean? I don't see why it's unclear.
There was a problem hiding this comment.
When we introduce XHR we should an example of how it's used, but that example is synchronous so it doesn't use onload. We never actually show an example with onload so someone who is not familiar with the XHR API might not understand what onload is or be missing the API to register an event handler.
| # ... | ||
| self.render() |
There was a problem hiding this comment.
Unchanged so I don't know why this is here.
| @@ -328,6 +308,20 @@ class TaskRunner: | |||
| self.condition.release() | |||
| ``` | |||
|
|
|||
| It's important to call `wait` at the end of the `run` loop if there is nothing | |||
There was a problem hiding this comment.
I thought this was weirdly placed (it’s a move I think)
There was a problem hiding this comment.
But not critical.
| @@ -1016,8 +1015,9 @@ is really being spent rendering. It's important to always measure | |||
| before optimizing, because the result is often surprising. | |||
|
|
|||
| To instrument our browser, let's have it output the | |||
| [JSON][json] tracing format used by [chrome://tracing][chrome-tracing] | |||
| in Chrome, [Firefox Profiler](https://profiler.firefox.com/) or | |||
| [JSON][json] tracing format used by | |||
There was a problem hiding this comment.
Why make this change? Seems not significant.
Not critical in any case
There was a problem hiding this comment.
Oh yeah, it looks weird to say "foo bar [12] in Chrome, Foo Bar [13], Foo Bar [14]". If we move "in Chrome" earlier the reference-comma becomes a consistent pattern and helps parsing because the references are big and commas are small.
There was a problem hiding this comment.
Not critical.
| @@ -1409,7 +1408,7 @@ class Chrome: | |||
| Event handlers are mostly similar, except that we need to be careful | |||
| to distinguish events that affect the browser chrome from those that | |||
| affect the tab. For example, consider `handle_click`. If the user | |||
| clicked on the browser chrome (meaning `e.y < self.chrome.bottom`), we can | |||
| clicked on the browser chrome, we can | |||
There was a problem hiding this comment.
Ah—It breaks across lines in a weird way and seems not critical so rather than rephrase text around it to avoid the weird line break I figured it's easier to delete this text. I think the relevant code used to be "e.y < 100", before we had Chrome do proper font-based layout, and back then the line of code was more cryptic, but I think now it's self-explanatory enough that we don't need to explain it.
| @@ -1818,7 +1817,9 @@ but in this case we don't know the right scroll offset yet. We need | |||
| the main thread to run in order to commit a new display list for the | |||
| other tab, and at that point we will have a new scroll offset as well. | |||
| Move tab switching (in `load` and `handle_click`) to a new method | |||
| `set_active_tab` that simply schedules a new animation frame: | |||
| `set_active_tab` that simply schedules a new animation frame:^[Note | |||
This one had several critical fixes, including one critical comment (see doc).