search

Netflix / vmaf

Perceptual video quality assessment based on multi-method fusion.
Other
4.34k stars 735 forks source link

Faster process pool #1369

Closed 101arrowz closed 3 weeks ago

101arrowz commented 3 weeks ago

Improves the performance of the process pool by replacing the polling-based approach with Python's built-in process pool executor, which reduces synchronization overhead and reuses the Python interpreter when possible instead of spinning up a new instance for every single task.

nilfm99 commented 3 weeks ago

I think the build failures are on Github's side, not ours. See https://github.com/actions/runner-images/issues/6131 and https://www.githubstatus.com/

101arrowz commented 3 weeks ago

The build failures seem to be due to an internal error with GitHub Actions, but I noticed there's a test for Windows support. As mentioned in the diff, neither the old implementation nor the new one works with kernels that do not support POSIX fork(), NT included. However, the old implementation would silently use a spawn-based method (and might thereby fail while pickling the arguments), while the new one fails fast due to an assertion. In the specific situation that all arguments are pickleable and the code is running on Windows, however, the old code would work fine while the new version does not.

nilfm99 commented 3 weeks ago

I think this should be okay, the python wrapper is secondary to the C library IMO, and Linux/Mac is our main focus. Tagging @christosbampis, @krasuluk and @li-zhi who are the main users of the python library in case they have objections.