[PATCH] D110018: [lld-macho] Speed up markLive()

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added inline comments.


================
Comment at: lld/MachO/MarkLive.cpp:171-172
+
+    if (start < idx || end >= size) {
+      if (idx < size) {
+        start = end = idx;
----------------
int3 wrote:
> oontvoo wrote:
> > int3 wrote:
> > > i'm kind of confused here. why are we comparing `start` to `idx` -- isn't `start` an index into the `size` array of jobs, whereas `idx` is the index of the `Task` vector of size `poolSize`? seems like we're comparing numbers on different scales...
> > > 
> > > same thing for `idx` and `size`
> > "idx" is the index of the Task vector, yes but it correlates to where its work items are:
> > 
> > So the division of labour is like this:
> > 
> > ```
> > Task[0] : start=0, end = start + d -1 = d -1 
> > Task[1] : start=d, end = d + d - 1
> > Task[2]: start = d + d, end = d + d + d -1
> > ....
> > Task[n]: start = n * d, end = start + d -1.
> > ```
> right but... what does `start < idx` mean here, and when do we expect it to be true? shouldn't it never be true, since `n >= n * d` for all nonnegative `n` and `d`?
Ah- I misunderstood what you were confusing about. 

>  shouldn't it never be true, since n >= n * d for all nonnegative n and d

d could be zero, which is non-negative :) 

ie., when we have fewer tasks than the number of workers, then you dont need to use all of the workers, when that happens, you just need the first [0, x) workers, and give each of them 1 task.

Similarly,  `end >= size` is true when `start== d == 0` (because its type is size_t , hence the `-1` will ensure it).


(but I guess you're right that this is written in a much more convoluted way than it should be ... will fix)



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110018/new/

https://reviews.llvm.org/D110018