search

JuliaSIMD / Polyester.jl

The cheapest threads you can find!
MIT License
240 stars 13 forks source link

README improvement #125

Closed carstenbauer closed 10 months ago

carstenbauer commented 10 months ago

See #124.

I've tried to clarify the README and have updated/rerun the examples on Julia 1.9 on a linux machine with 8 threads.

The sections about threadlocal and disabling polyester threads have not gotten much attention (but benchmarks in the latter have been rerun as well).

(I think the package needs an overhaul but this is beyond this PR which just tries to communicate the status quo a bit better.)

chriselrod commented 10 months ago

I think the package needs an overhaul but this is beyond this PR which just tries to communicate the status quo a bit better.

What do you have in mind?

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (f29a42e) 91.13% compared to head (7832539) 91.13%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #125 +/- ## ======================================= Coverage 91.13% 91.13% ======================================= Files 3 3 Lines 451 451 ======================================= Hits 411 411 Misses 40 40 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

carstenbauer commented 10 months ago

What do you have in mind?

Apart from what I said above - I'd personally drop auto @inbounds and view behavior - threadlocal is a misnomer and very much broken. Also per is somewhat of a misnomer to me. Additionally, the disabling of polyester threads doesn't show the timing differences anymore that it used to which warrants a reconsideration of this section/feature. Statements about nesting should be generally revisited given that @threads defaults to :dynamic these days, which is nestable with ok-ish performance.

chriselrod commented 10 months ago

Thanks!

I don't really want to change PtrArray. I do not like copy-by-default, so even though it is now documented that slices of AbstractArrays are supposed to copy, maybe I'll make PtrArray no longer subtype AbstractArray. We could drop the StrideArraysCore dependency and just make our own PtrArray type for being able to pass it to the tasks.