search

acdlite / react-fiber-architecture

A description of React's new core algorithm, React Fiber
11.6k stars 590 forks source link

Why is the numerical value of "pendingWorkPriority" inversely proportional to its meaning? #8

Open Prinzhorn opened 7 years ago

Prinzhorn commented 7 years ago

This line is the only one I had to read multiple times (> 2) to understand it. Because it went against my intuition and I had to make sure I read that correctly.

With the exception of NoWork, which is 0, a larger number indicates a lower priority

Shouldn't a higher priority be represented by a higher number? The way it's currently proposed sounds more like a pendingWorkNiceness than a priority. I believe this will result in unnecessary mental overhead now and in the future. Every time someone touches code or reads about something related to priorities it causes this twist in the brain. For example the pseudo code "to check if a fiber's priority is at least as high as the given level" compares it with the given level using <= (read: smaller than or equal then the given). I believe someone implementing the code would first use >=, then run the tests and go "oh right, higher means lower", facepalms for second and goes on.

Given that NoWork is the lowest priority, so low that it will never be scheduled, has a value of 0 (and not +Infinity), makes it even more complicated.

I'm curious what led to this decision. Apart from that cheers for the concepts behind Fiber, sounds a lot like process scheduling on a CPU mixed with some dynamic programming ✌️.

mquandalle commented 7 years ago

This is indeed a potential confusion that we see in the current code: https://github.com/facebook/react/blob/f53854424b33692907234fe7a1f80b888fd80751/src/renderers/shared/fiber/ReactFiberScheduler.js#L427

I’m also curious about this design decision.

abigsmall commented 7 years ago

I was confused by this too until I thought of it as the priority that’s assigned to bugs. a P1 bug would be of the “all hands on deck” type, whereas a P5 is “meh, we’ll probably never get around to it”. see here for one description of priority bug rankings.

Prinzhorn commented 7 years ago

I was confused by this too until I thought of it as the priority that’s assigned to bugs. a P1 bug would be of the “all hands on deck” type, whereas a P5 is “meh, we’ll probably never get around to it”. see here for one description of priority bug rankings.

@abigsmall Except for NoWork, which should be 6 following this logic ;)

abigsmall commented 7 years ago

@Prinzhorn nah, the context of the logic is “the priority that’s assigned to bugs”. a P5 bug still has a modicum of possibility of being fixed, whereas a bug that will absolutely not be fixed will not be given any priority at all. I think that falls in line with a priority value of 0, essentially. perhaps a bit of a stretch, but it makes sense to me :)