JuliaAttic / CUDArt.jl

Julia wrapper for CUDA runtime API
Other
79 stars 29 forks source link

Fixup build steps for windows and misc code refactoring #77

Closed musm closed 7 years ago

musm commented 7 years ago

I see no improvements in precompile speed using snoop precompile.

build.jl can be made better but at least this is a first step... most of the build is copied over from CUDAdrv cc @maleadt

musm commented 7 years ago

anyone know why the Julia 0.5 build bot is using Julia Version 0.5.2-pre+1 and not the release version? I don't think that is recommended.

timholy commented 7 years ago

Snoop precompile files don't speed up building a package (quite the contrary, in fact); they (might) decrease the latency to use the package. If you build rarely but use often, this can be a win.

musm commented 7 years ago

I didn't notice any increase in speed when doing using CUDArt , what's the typical speed up you observe? If it's negligible, I think it's worth it to remove the additional precompile files as it doesn't add much.

timholy commented 7 years ago

It's not for using CUDArt, it's latency to the first "run some functions and give me a result."

That said, it's entirely possible that things have changed with base julia since those were defined; I'm not saying it's bad to delete src/precompile.jl, I just want to make sure you're measuring against its intended purpose. If you can't see any kind of reduction in latency, measured properly, then by all means delete it.

maleadt commented 7 years ago

@musm

anyone know why the Julia 0.5 build bot is using Julia Version 0.5.2-pre+1 and not the release version? I don't think that is recommended.

The buildbot not only tests packages when triggered by PRs/pushes, but also tests when commits are detected on Julia release branches in order to spot regressions. This is managed by a unified configuration. Sure, it would be ideal to have that first type of testing use tagged versions, but I haven't run into issues using top-of-release_$VER (even though it's not recommended). So I'd leave it at how it is, until it becomes a pressing issue | I have time to fix it.

maleadt commented 7 years ago

It might also be interesting to move the compilation example from CUDAdrv over to CUDArt, because CUDArt already requires the entire toolkit to be installed. It would require dumping some of the detected toolchain information to eg. ext.jl.

That saved information could then also be used to check if it is even necessary to rebuild CUDArt (eg. how it is done in CUDAdrv).

musm commented 7 years ago

@timholy I'm not seeing any perf increase using snoopcompile for functions. I won't remove the files since I may not be testing things rigorously enough to prove that it doesn't have benefits. Someone else should double check however.

timholy commented 7 years ago

Well, now that you understand its intended purpose, if you're still seeing no benefit then perhaps it's not useful on more recent julia versions. I'd say do whatever seems sensible.

musm commented 7 years ago

As far as I'm concerned this pr is sufficient for review now. Tests are passing on windows locally too.

musm commented 7 years ago

btw this is g2g on my end. should be good enough for now. A refactor and better cohesion with CUDAdrv would be nice but outside the scope of this PR. At least builds should now work on all platforms.

vchuravy commented 7 years ago

Thank you! Can you squash the commits?

vchuravy commented 7 years ago

Thank you for the work!