gronke / py-jail

A Python libc wrapper for FreeBSD jails
Other
4 stars 2 forks source link

use ctypes.CDLL() directly instead of find_library #3

Closed igalic closed 4 years ago

igalic commented 4 years ago

This should be more efficient, as it doesn't need to call ldconfig, gcc, objdump or ld to determine its existence. ctypes.CDDL() will use dlopen() directly.

Note that we're hard-coding the current version (libc.so.7). This will need changing when FreeBSD changes their ABI.

At that point we might have to rethink our strategy, or fix ctypes.util.find_library() to be less awful. Until then, this should give us a minimal performance boost.

gronke commented 4 years ago

@igalic you got a point there. I've compared the time it takes to lookup the library to the time it takes to actually import it.

# python3.7 /tmp/cdll-perf.py
find_libc took 2.3949146270751953 ms
import_libc took 0.06580352783203125 ms
Importing took 2.7476% of the time to find libc

This is the script I used for comparing the two actions:

import time
import ctypes
import ctypes.util

def find_libc():
    ctypes.util.find_library("c")

def import_libc():
    ctypes.CDLL("libc.so.7")

def measure(action):
    start = time.time()
    action()
    end = time.time()
    return (end - start) * 1000

time_find = measure(find_libc)
time_import = measure(import_libc)

print(f"find_libc took {time_find} ms")
print(f"import_libc took {time_import} ms")
percent = round(100 / time_find * time_import, 4)
print(f"Importing took {percent}% of the time to find libc")

Reducing the lookup time by 97% is quite significant 👏

What bugs me though is that we bypass the intended way to dynamically look up the library.

Note that we're not hard-coding the current version (libc.so.7) since on FreeBSD a symlink is in place.

I did not find such symlink, see:

# diff /usr/lib/libc.so /lib/libc.so.7 
Binary files /usr/lib/libc.so and /lib/libc.so.7 differ

This (older) post addresses this issue as well: https://lists.freebsd.org/pipermail/freebsd-python/2015-February/008018.html

How do we make sure that libc.so.7 is always the one we want (future compatible)? I see that there are gains in the range of milliseconds. But is it worth risking compatibility issues?

igalic commented 4 years ago

oof, i've updated the commit message (here) but not the PR message:

Note that we're hard-coding the current version (libc.so.7). This will need changing when FreeBSD changes their ABI.

At that point we might have to rethink our strategy, or fix ctypes.util.find_library() to be less awful. Until then, this should give us a minimal performance boost.