Revert "partr: fix multiqueue resorting stability"

[JeffBezanson]

This reverts commit c23554a.

This caused pfib to run ~forever.

[vtjnash]

Hm, any idea why? I know I split out that commit so it could be bisected (and reverted) easily, but it'd be good to understand

[JeffBezanson]

All the tasks have the same priority, so I think this was causing it to iterate through most of them on each operation.

[JeffBezanson]

@kpamnany What is the right thing to do here to dequeue tasks in the right order without being O(n^2)?

[kpamnany]

@JeffBezanson: What's the 'right' order? Originally partr used <= in both sift_up() and sift_down(). So in multiq_insert(), the most recently added task was moved to the top of the heap and after a multiq_deletemin(), the task from the bottom of the heap moved back down to the bottom. This honors the depth first strategy.

Looks like this has been changed to < in both... but not sure why.

[JeffBezanson]

Looks like this has been changed to < in both... but not sure why.

Because using <= causes the code to traverse a large part of the structure on each operation, making the scheduler O(n^2) and then we spend all of run time there.

[kpamnany]

Have you confirmed that with profiling? If not, I should be able to finally get started on doing some of that this week.

It shouldn't really happen that sifting consumes a lot of runtime. There are 2 * jl_n_threads heaps that are 8-ary trees laid out breadth-first, i.e. 8 task movements in a single cache line. If you have a workload that creates a lot of tasks which yield repeatedly, then perhaps it could get slow -- that's a bad profile for this approach.

If you increase heap_c to 4 here you'll get some speedup for these workloads, but there'll be some overhead when there aren't many tasks.

[JeffBezanson]

Yes. Try running this:

function pfib(n::Int)
    if n <= 1
        return n
    end
    t = Threads.@spawn pfib(n-2)
    return pfib(n-1) + fetch(t)::Int
end
pfib(25)

with 2 threads.
With <:

  0.176900 seconds (729.64 k allocations: 87.096 MiB, 12.04% gc time)

with <=:

  6.887915 seconds (728.51 k allocations: 87.061 MiB, 0.49% gc time)

I admit this is a pathological case, but I don't think it's ok for the default scheduler to have any worst case this bad.

[kpamnany]

Yes. There's a 7.5X slowdown with fib in the C partr as well. And we aren't even using the priorities right now AFAICT. This needs some thought.