treeform / puppy

Puppy fetches via HTTP and HTTPS
MIT License
185 stars 27 forks source link

[question] can puppy be {.gcsafe.}? #15

Closed veksha closed 2 years ago

veksha commented 3 years ago

Hi. thanks for this neat library. is it possible to make it gcsafe? so we can use it with "parallel block" (do a lot of requests in parallel). I'm using {.cast(gcsafe).} block to disable compiler errors and I think it is working fine (not tested on a real project yet)

{.experimental.}
import threadpool, puppy, std/times

proc download() =
    {.cast(gcsafe).}:
        let req = Request( url: parseUrl("https://google.com"), verb: "get")
        discard fetch(req)

let t1 = cpuTime()
parallel:
    for i in 0..12:
        spawn download()
let t2 = cpuTime()
echo "1: ",t2 - t1, " (parallel)"

let t3 = cpuTime()
for i in 0..12:
    download()
let t4 = cpuTime()
echo "2: ", t4 - t3
# nim c --threads:on -r test

1: 0.401 (parallel)
2: 2.253
theAkito commented 3 years ago

Are you using this for a Windows app?

veksha commented 3 years ago

yes, I am planning to use it for a windows app. is it possible technically to parallelize requests correctly on windows with help of puppy lib?

theAkito commented 3 years ago

https://github.com/khchen/winim/issues/71

I wanted to create a pull request for this library, to rewrite stuff and make it better. However, this does not solve the issue linked above.

As you see, the Windows thing has some GC safety bug, as pointed out by winim's great author.

So, if you want to have a workaround for the GC safety, then you have to apply the workaround I lined out in that linked issue.

veksha commented 3 years ago

So, if you want to have a workaround for the GC safety, then you have to apply the workaround I lined out in that linked issue.

@theAkito, can you tell me what do you mean by this:

Somehow, no matter how I tried to use it like that, it never worked with more than a single line. If I added a second line, or more, then it always errored out.

does it mean {.cast(gcsafe).}: block is not working for you? (did you forget "cast"?)

treeform commented 3 years ago

One needs to figure make sure its gcsafe for both the windows api and the linux/mac libcurl api. libcurl api does strange things with strings.

theAkito commented 3 years ago

Didn't use Linux thing, yet. But the Windows thing was pretty broken.

theAkito commented 3 years ago

does it mean {.cast(gcsafe).}: block is not working for you? (did you forget "cast"?)

Nothing of that sort worked. It simply did not accept multiple lines, only single ones. Just as I described in the linked issue.

That, however, does not matter much. The actual problem is the specious lack of GC safety.

guzba commented 2 years ago

Puppy no longer has any dependencies which were the source of the concern here. All 3 platform fetch implementations now have also been used and tested in a threaded context: https://github.com/treeform/puppy/blob/master/src/puppy/requestpools.nim and https://github.com/treeform/puppy/blob/master/tests/test_requestpools.nim

This should now work and be safe, excluding the usual pile of threading concerns that cannot be avoided. Please re-open if there are concerns or bugs.