threadid() and nthreads() usage

[cormullion]

From this blog post we read that:

Any usage of threadid() in package or user code should be seen as a warning sign that the code is relying on implementation details, and is open to concurrency bugs.

In Luxor threadid() does appear a few times:

• Luxor.jl/src/drawings.jl

  Line 90 in 18d96c8

  id = Threads.threadid()

• Luxor.jl/src/basics.jl

  Line 553 in 18d96c8

  id = Threads.threadid()

so there might be the possibility of issues and/or bugs.

[hustf]

Oops, we ought to think this through. I have used the thread features some on Julia 1.9 and did not notice any issues.

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[guo-yong-zhi]

Oops, we ought to think this through. I have used the thread features some on Julia 1.9 and did not notice any issues.

I think I found some issues: #321

[guo-yong-zhi]

I have a proposal to handle the Luxor context problem of parallel programming. It is to allow the user to pass a context object to all the relevant functions. I admit that it would be a big refactoring, but it will be worth it.

Here is my proposal:

At first, we should define (within Luxox) a new type for the context object, say Context. The Context object should contain all the information that used to be in the global space. For example (pseudo code):

struct Context
    saved_colors::Tuple{Float64, Float64, Float64, Float64}[]
    CURRENTDRAWING::Array{Drawing, 1}()
    CURRENTDRAWINGINDEX::Ref{Int}
end
function drawing_context(*a)
    ctx = Context([], [], Ref(0))
    push!(ctx.CURRENTDRAWING, Drawing(*a))
    return ctx
end

Secondly, we should add a new verison method that takes a Context object as its first (or last?) argument for every drawing function. That would be a heavy work if we manually do that for all the functions. But I guess it can be done automatically with some metaprogramming tricks. By the way, I think the Context version of the function should be the hand-written one.

After that, users can do the following:

ctx = drawing_context(1000, 1000)
origin(ctx)
fontsize(ctx, 50)
text(ctx, "hello world")
finish(ctx)

Additionally, if we also have a magic macro, say @context, to do the transformation, the code will be much easier to write and just look like the old un-parallel verison code.

@context 1000 1000 begin
    origin()
    fontsize(50)
    text("hello world")
    finish()
end

---

The old threadid() based global context mechanism could be preserved and refactored using the new Context object. So, this new mechanism won't break anything. It will just make the Luxor @spawnable and more manageable I believe.
If I missed something, please let me know.

[cormullion]

Whatever its merits, I think this change is too big, and goes against the spirit of Luxor being simple (and not using Cairo contexts…!). I would prefer to see Luxor being single-threaded than becoming more complex.

Perhaps Makie.jl is a more suitable location for more complex multi-threaded code.

[guo-yong-zhi]

Here is another solution that can make Luxor @spawnable. It works on the similar idea of the previous one, but much simpler.
Here is my new proposal:
At first, we should add a new field to the Drawing object, say saved_color_stack, which is a alternative to the global _SAVED_COLORS_STACK.

mutable struct Drawing
    width::Float64
    ...
    strokescale::Bool
    saved_color_stack::Vector{NTuple{4,Float64}}
    ...

Secondly, we should rewrite every drawing function. Pass a Drawing object to the function as its keyword argument, and set the global implicit Drawing object as the default value, says, ...; drawing=_get_current_drawing_save(). It is compatible with the old api and doesn't break anything. Here is some example code:

function origin(pt; drawing=_get_current_drawing_save())
    setmatrix([1.0, 0.0, 0.0, 1.0, 0.0, 0.0]; drawing=drawing)
    Cairo.translate(drawing.cr, pt.x, pt.y)
end
function gsave(; drawing=_get_current_drawing_save())
    Cairo.save(drawing.cr)
    r, g, b, a = (drawing.redvalue, drawing.greenvalue, drawing.bluevalue, drawing.alpha)
    push!(drawing.saved_color_stack, (r, g, b, a))
    return (r, g, b, a)
end

After that, users can do the following:

function my_spawnable_function(i=0)
    filename = "$i.png"
    d = Drawing(1000, 1000, filename)
    origin(; drawing=d)
    fontsize(50; drawing=d)
    text(filename; drawing=d)
    finish(; drawing=d)
end

fetch.(Threads.@spawn my_spawnable_function(i) for i in 1:10)
# old-style function counterpart that without `drawing=d` will only output 4 pngs if you launch julia with `julia -t 4`, 
# whatever in old version Luxor or new proposal version here.

Again, a convenient macro @context can be added to do the transformation, just as I mentioned in the previous post.

---

Although it's not a breaking change, it is still a big change. Almost all the functions need to be rewritten.
So if you guys think the @spawnablity is not that important in Luxor, and don't want anybody to change the original mechanism this way, I will understand. (However, I believe that parallel computing will be more and more common in the future. I am not sure if Luxor is the right place to do that.)
On the other hand, since it is a very heavy and tedious refactoring, I wouldn't be surprised if nobody (including me) wants and has time to do that, even if given the permission to.