We've had some internal discussions about adding folly as a dependence, which also requires boost. This should allow us to get better performance in compilation and runtime, and make some things easier/cleaner. @bertmaher has put up a PR to test this out -- see https://github.com/pytorch/glow/pull/4051
Hoping to get some feedback from the community -- please let us know any concerns you may have. CC: @mciprian13 @tlepley-cadence @ayermolo @aturetsk @vuzelac-cadence @jsubag @et-nivard @mneilly-et @wyessen
A particular concern is cross-platform compatibility. In theory folly works on Linux, macOS, and Windows, but it鈥檚 most thoroughly tested on Linux... and in terms of the build system I don鈥檛 even know how to test Windows! So I鈥檇 love someone to test drive this PR to flush out issues. Thanks!
That is my concern also.
Also that is quite a few new dependencies. Including openssl. Since this is an FB project, anyway it can be changed so that not everything needs to be imported?
From shipping to customers perspective fewer external dependencies and third party libraries that are included the better. I would think.
@ayermolo totally get your concerns. From FB鈥檚 internal perspective dependences are basically no problem, so there hasn鈥檛 been any effort made to split things out of folly at a finer granularity.
It turns out the compression libraries are optional, which does remove a few deps (snappy, xz, lz4), but it鈥檚 still not nothing.
Which deps are particularly objectionable? I can attempt some tricks to slice out the worst offenders. another project I鈥檝e worked on basically selects bits of folly on a file level, though I had hoped to avoid that since it鈥檚 annoyingly to maintain. But we can do it in a pinch.
@ayermolo One more question: do you need Glow's runtime for your customers? We can easily make folly a dependence only for the runtime component libraries, so you wouldn't need folly unless you also want the runtime.
As a general thing I think keeping number of dependencies/third party packages that are not used by glow to few as possible is better. It reduces the space of things not integrating well with various environments and build tool chains. Specially considering there is no CI for WIndows.
Besides those other main one that jumped out was openssl.
Well we do ship apps to customers that use just enough of runtime to run CPU backend floating point.
I agree with @ayermolo - packages not used should go out. We see that might be a hard thing to do, but is it possible to put effort to get Windows CI build ?
For example this:
_@bertmaher I鈥檇 love someone to test drive this PR to flush out issues._
This is actually a continuous work - anything we do to fix the Windows build might be broken soon after. Just that with the folly lib, we expect it to be harder to make it keep running.
I would also feel more comfortable about new dependencies if the Windows CI build would be up and running. Currently we are providing Glow support for some clients using Windows build based on a Msys2 MinGW environment (along the lines of this doc).
Fleshing out our options here, does it improve things if we "vendor" the extra deps by adding them to third_party and building Glow monolithically? That approach reduces the dependency on the base system (tensorflow does this pretty extensively), but obviously has its own problems (initial build is really slow, maybe difficult to set up some of the packages).
We can maybe subset folly to exclude openssl. That would get the list of additional dependences down to: boost, double-conversion, jemalloc, libevent. Which is pretty much the set of transitive things we actually want. Is that an acceptable subset?
As a last resort, we can maybe maintain a separate runtime for our perf-sensitive needs, and include folly optionally, but that's not an ideal state either.
Windows CI is kind of its own thorny issue. I personally don't have the bandwidth to set up Windows CI for Glow, and I don't know if anyone else on my team does either. folly itself goes through internal CI on Windows so it's probably not going to break frequently, but I don't blame you if that statement doesn't give you warm, fuzzy feelings ;-)
@bertmaher Thanks for looking in to it. I think excluding openssl and getting dependency down to boost, double-conversion, jemalloc, and libevent should be enough. @vuzelac-cadence ?