Skip to content
This repository has been archived by the owner on Jun 2, 2022. It is now read-only.

Cache OpenCL binaries on BSDs #217

Merged
merged 1 commit into from
Jan 3, 2020
Merged

Cache OpenCL binaries on BSDs #217

merged 1 commit into from
Jan 3, 2020

Conversation

jbeich
Copy link

@jbeich jbeich commented Dec 16, 2019

  • KERN_PROC_PATHNAME is only used when running from build directory, ~/.waifu2x otherwise
  • sysctl either fails or succeeds but never truncates buffer, so realloc is rarely called more than once
  • If getting path length fails then executable has no name i.e., unlinked

@@ -313,7 +316,8 @@ namespace w2xc
const char *dev_name = proc->dev_name;
bool bin_avaiable = false;

#if ((defined __linux) && !(defined __ANDROID__)) || _WIN32
#if ((defined __linux) && !(defined __ANDROID__)) || defined(_WIN32) || \
defined(__DragonFly__) || defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__NetBSD__)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there no cleaner way to check for *BSD variants than a million defined checks ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defined(BSD) but it's only defined if <sys/param.h> is included, haven't been updated since BSD 4.4 Lite2 and is also defined on macOS. sysctl code is kernel-specific e.g., KERN_PROC_PATHNAME isn't supported on OpenBSD (for security reasons) or macOS (uses _NSGetExecutablePath); while /proc has been removed on DragonFly + OpenBSD and deprecated on FreeBSD + NetBSD.

Does current version look better?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, GENERATE_BINARY can probably be enabled on all platforms. If getting absolute path to executable isn't supported/implemented current directory can be used as a fallback.

@koachan
Copy link

koachan commented Dec 28, 2019

Yes. I remembered Solaris (and derivatives) can be supported [...]

AFAICT the two libraries needed for this app to run (OpenCL and OpenCV) isn't compatible with Solaris, so IMHO it's cleaner to not add any __sun-specific code for now.

... or should we?

@jbeich
Copy link
Author

jbeich commented Dec 28, 2019

OpenCV (3.4.8 atm) is available in PkgSrc. OpenCL is not packaged by PkgSrc (NetBSD, Solaris) or OpenBSD Ports.

With OpenCL the hard part is getting modern graphics drivers e.g.,
https://wiki.openindiana.org/oi/Intel+KMS+driver

Userland is easier. For one, I've ported Intel NEO to FreeBSD without relying on platform-specific stuff. DragonFly picked up a few days later by nature of sharing package recipes.

@DeadSix27
Copy link
Owner

I'm gonna be that guy but, with all these limits and headaches to work around are there even any users of this program and or interested parties?

@jbeich
Copy link
Author

jbeich commented Dec 28, 2019

FreeBSD and DragonFly have waifu2x-converter-cpp (since 2015) and waifu2x-ncnn-vulkan (unusable on NVIDIA) in packages. For one, I've used waifu2x-converter-cpp --force-OpenCL (with Beignet on Skylake) on FreeBSD for a few years before switching to waifu2x-ncnn-vulkan for better performance and newer models. Telemetry is opt-in and not popular outside of desktop-oriented distros e.g., I don't enable to avoid spoiling stats for my packages.

@jbeich
Copy link
Author

jbeich commented Dec 28, 2019

Dropped Solaris bits. Anything else?

@DeadSix27 DeadSix27 self-requested a review December 30, 2019 09:58
Copy link
Owner

@DeadSix27 DeadSix27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTMN (Did not test on BSD, nor can. Up to you on that one, but the code looks fine.)

Tell me if it's good to go then I'll merge.

@DeadSix27 DeadSix27 merged commit ad2a3ee into DeadSix27:master Jan 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants