Fix node v12.0.0 errors, update to nan v2.13 #69
Conversation
…eseven/node-opencl into feature/upgrade_to_NAN_2.8.0
…ntal OpenCL 2.1 CPU Only Platform
* return compiled program from clGetProgramInfo(cl.PROGRAM_BINARIES) * detect if a buffer is being unwrapped inside NoCLWrapper::Unwrap, and reinterpret_cast into the type if so * only pass programs with size > 0 to clCreateProgramFromBinary for nvidia
…h older node versions
|
Thank you very much! |
|
Hey @trxcllnt But I see that the last commit already contains mentioned fix from you here c64da8d ? However backward compatibility is not an issue for me. Is this repo live at all? All this work in this PR is precious but is invisible to everyone, such a waste. I've incidentally dropped in here and was happy to find it. Maybe it's time for a new maintainer or even project? |
|
@lu4 oh, you're right! I guess I did the find/replace when I first figured it out, then forgotten I did it. As to the other thing, you'd have to ask @mikeseven. I won't be able to contribute much further from here on out, but I'd be happy to help with any changes necessary to get this PR merged. |
|
Since the maintainer is not active I'll probably try to somehow fork or spin-off new project based on this work (as a temporary solution) I'll try to include all the PRs suggested by the community as per my understanding of BSD-3 license ( https://tldrlegal.com/license/bsd-3-clause-license-(revised) ) it should be fine |
|
Also having a TypeScript support would be great |
|
@trxcllnt JFYI, I've noticed that the contents of some |
|
@trxcllnt I can create PR for your fork? |
|
Here it is: trxcllnt#1 |
Add missing Nan support, for sampler.cpp, less than OpencCL version 2
* dev/fix-tests: fix(tests) Based on mikeseven@3603302 Minor code quality improvements
|
Extra waiting commit: trxcllnt#3 We're kind of stuck now. We'll maintain http://github.com/graphistry/node-opencl#next as our node 12 prod and I'll ping Mike about what's happening here. |
|
Great, I have commit access now -- merging. (Already been using in prod for graphistry.) |
|
NOTE: Asking Mike on how to push a version update to npm, so almost there. |
Hi @mikeseven! Here's a PR with updates to make everything compile in node v12.0.0. It includes a few small fixes (like 3e54df5), and I updated a bunch of tests where appropriate. I've tested with the Intel Core i7 8700 and nvidia GTX 1070s in my laptop, but will also be testing on the V100s in a little bit.
It also adds a feature to return Uint8Arrays of the program binaries from
cl.getProgramInfo()(test here). I updated theNoCLWrapper::Unwrapmethod to callNewInstanceif theLocal<Value>is a Uint8Array. This was the easiest way I found to makecl.createProgramFromBinary()work when it's passed a list of Uint8Arrays.Look it over and let me know what you think. I still need to update our other dependencies for node 12 so I haven't tried in production settings yet, but I'll update this PR if I run into anything. Once this looks good and is merged, do you think we could do a new release of node-opencl to npm? We've been referencing my fork via a git hash, but it'd be great to have an official v0.5.0 out on npm.