Merge lp:~gz/bzr/trivial_urandom_uncalled into lp:bzr

[Martin Packman]

Simplify wrapping of os.urandom done by osutils

Peel back some of the layering around os.urandom done by osutils at module start time. It's worth avoiding reading the one byte to check the function works on windows, as the first call does expensive initialisation, notably advapi32.dll needs loading. That makes this one call account for 50-100ms on my windows box, where a simple command takes in the region of 400ms on hot caches.

With --profile-imports the change is from:
    143.5 103.6 ++ [osutils]bzrlib @ bzrlib.plugin:36
To:
     38.7 4.4 ++ [osutils]bzrlib @ bzrlib.plugin:36

The various users of osutils.rand_chars (fancy_rename, lockdir) mean commands that do real work will still need to pay this at a later stage.

Various notes:
* AttributeError from accessing 'urandom' on os not needed since Python 2.4
* NotImplementedError from nt.urandom not an issue with any supported windows versions
* Wrapping /dev/urandom serves no purpose since Python 2.4
* isinstance(os.urandom, types.BuiltinFunctionType) would be a better check but needs an import

Are there in fact any circumstances in which /dev/urandom will not be available that we want to support, or should I just go all the way and remove the Mersenne Twister fallback as well?

[Jelmer Vernooij (jelmer)]

> Are there in fact any circumstances in which /dev/urandom will not be available that we want to support, or should I just go all the way and remove the Mersenne Twister fallback as well?

I have no idea what you mean by "the Mersene Twister fallback". Other than that, I think this is a reasonable change.

review: Approve

[Martin Packman (gz)]

> I have no idea what you mean by "the Mersene Twister fallback". Other than
> that, I think this is a reasonable change.

Using the random module (which gives pseudo-random output). Following the (mostly unrelated) python-dev thread, it seems a guess for one circumstance where this might be needed was right, in a chroot without /dev exposed (which is arguably a broken setup). If they go ahead with randomising the hash function on startup, this change won't be a win any more, but never mind.

<http://mail.python.org/pipermail/python-dev/2012-January/115263.html>

[Martin Packman (gz)]

sent to pqm by email

=== modified file 'bzrlib/osutils.py'
--- bzrlib/osutils.py	2012-01-02 08:22:01 +0000
+++ bzrlib/osutils.py	2012-01-03 17:51:38 +0000
@@ -945,19 +945,16 @@
     return os.fstat(f.fileno())[stat.ST_SIZE]
 
 
-# Define rand_bytes based on platform.
-try:
-    # Python 2.4 and later have os.urandom,
-    # but it doesn't work on some arches
-    os.urandom(1)
-    rand_bytes = os.urandom
-except (NotImplementedError, AttributeError):
-    # If python doesn't have os.urandom, or it doesn't work,
-    # then try to first pull random data from /dev/urandom
+# Alias os.urandom to support platforms (which?) without /dev/urandom and 
+# override if it doesn't work. Avoid checking on windows where there is
+# significant initialisation cost that can be avoided for some bzr calls.
+
+rand_bytes = os.urandom
+
+if rand_bytes.__module__ != "nt":
     try:
-        rand_bytes = file('/dev/urandom', 'rb').read
-    # Otherwise, use this hack as a last resort
-    except (IOError, OSError):
+        rand_bytes(1)
+    except NotImplementedError:
         # not well seeded, but better than nothing
         def rand_bytes(n):
             import random