[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Nick Coghlan]

Nick Coghlan  added the comment:


New changeset 53efbf3977a44e382397e7994a2524b4f8c9d053 by Nick Coghlan in 
branch 'master':
bpo-11063: Handle uuid.h being in default include path (GH-4565)
https://github.com/python/cpython/commit/53efbf3977a44e382397e7994a2524b4f8c9d053


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Nick Coghlan]

Nick Coghlan  added the comment:

The header file check in setup.py incorrectly reported "not found" if `uuid.h` 
was in one of the standard include directories, so I've submitted a tweak to 
fix that: https://github.com/python/cpython/pull/4565

--
nosy: +ncoghlan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Nick Coghlan]

Change by Nick Coghlan :


--
pull_requests: +4493

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Berker Peksag]

Berker Peksag  added the comment:


New changeset 0e163d2ced28ade8ff526e8c663faf03c2c0b168 by Berker Peksag in 
branch 'master':
bpo-11063: Use more reliable way to check if uuid function exists (GH-4343)
https://github.com/python/cpython/commit/0e163d2ced28ade8ff526e8c663faf03c2c0b168


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Berker Peksag]

Berker Peksag  added the comment:

It worked for me on OS X (returns no) and Linux (returns yes after I installed 
uuid-dev) but I didn't test it on both systems at the same. Travis CI also 
returned 'no': https://travis-ci.org/python/cpython/jobs/299285001

In any case, Serhiy's suggestion is better than mine so I've opened PR 4343.

And yes, I'm beginning to regret my decision on not using AC_CHECK_LIB :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Berker Peksag]

Change by Berker Peksag :


--
pull_requests: +4301

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Stefan Krah]

Stefan Krah  added the comment:

On Wed, Nov 08, 2017 at 08:28:10PM +, Serhiy Storchaka wrote:
> Does this check work? I tried similar checks with other functions and they 
> were falsely passed because calling an undeclared function is not an error in 
> C.

Not here. If I replace uuid_generate_time_safe with a non-existing function
name, it is still found:

   checking for uuid_generate_time_unsafe... yes

The originally suggested AC_CHECK_LIB macro however works here in both cases. :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Does this check work? I tried similar checks with other functions and they were 
falsely passed because calling an undeclared function is not an error in C.

The reliable check of the existence of the uuid_generate_time_safe function is:

void *x = uuid_generate_time_safe

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Berker Peksag]

Change by Berker Peksag :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Berker Peksag]

Berker Peksag  added the comment:


New changeset 9a10ff4deb2494e22bc0dbea3e3a6f9e8354d995 by Berker Peksag in 
branch 'master':
bpo-11063: Add a configure check for uuid_generate_time_safe (GH-4287)
https://github.com/python/cpython/commit/9a10ff4deb2494e22bc0dbea3e3a6f9e8354d995


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[STINNER Victor]

STINNER Victor  added the comment:

"""

Unrelated to the patch (same before and after), this looks odd to me:

>>> import uuid
>>> uuid._has_uuid_generate_time_safe is None
True
>>> 
>>> import _uuid
>>> _uuid.has_uuid_generate_time_safe
1
"""

None means "not initialized yet". It's initialized on demand, at the first call 
of uuid1() or get_node():

$ python3
Python 3.7.0a2+ (heads/master:a5293b4ff2, Nov  6 2017, 12:22:04) 
>>> import uuid
>>> uuid._has_uuid_generate_time_safe  # == None
>>> uuid.uuid1()
UUID('3e5a7628-c2e5-11e7-adc1-3ca9f4650c0c')
>>> uuid._has_uuid_generate_time_safe
1


> [Also, I thought we weren't supposed to use ctypes in the stdlib.]

Antoine's commit a106aec2ed6ba171838ca7e6ba43c4e722bbecd1 avoids ctypes when 
libuuid is available.

For the other systems without libuuid, well, it was probably simpler to use 
ctypes. ctypes was more popular a few years ago. The code "just works" and I 
guess that nobody wants to touch it :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Stefan Krah]

Stefan Krah  added the comment:

Berker's latest patch looks good to me.

Unrelated to the patch (same before and after), this looks odd to me:

>>> import uuid
>>> uuid._has_uuid_generate_time_safe is None
True
>>> 
>>> import _uuid
>>> _uuid.has_uuid_generate_time_safe
1


[Also, I thought we weren't supposed to use ctypes in the stdlib.]

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Berker Peksag]

Berker Peksag  added the comment:

I've followed Stefan's suggestion and opened PR 4287 (tested on 10.10.5)

--
nosy: +berker.peksag

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Berker Peksag]

Change by Berker Peksag :


--
pull_requests: +4250

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Stefan Krah]

Stefan Krah  added the comment:

I think the configure check should be this (sets HAVE_LIBUUID in pyconfig.h):

diff --git a/configure.ac b/configure.ac
index 41bd9effbf..90d53c1b7d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2657,6 +2657,7 @@ AC_MSG_RESULT($SHLIBS)
 AC_CHECK_LIB(sendfile, sendfile)
 AC_CHECK_LIB(dl, dlopen)   # Dynamic linking for SunOS/Solaris and SYSV
 AC_CHECK_LIB(dld, shl_load)# Dynamic linking for HP-UX
+AC_CHECK_LIB(uuid, uuid_generate_time_safe)
 
 # only check for sem_init if thread support is requested
 if test "$with_threads" = "yes" -o -z "$with_threads"; then

--
nosy: +skrah

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Ned Deily]

Ned Deily  added the comment:

I agree with Barry's comment on PR 3855: "I'd rather see a configure check for 
the existence of uuid_generate_time_safe() rather than hard coding it to 
platforms !APPLE for two reasons. 1) If macOS ever adds this API in some future 
release, this ifndef will be incorrect, and 2) if some other platform comes 
along that doesn't have this API, it will still use the incorrect function."  
It's exactly for situations like this that autoconf tests exist; we should not 
be hardwiring assumptions about the lack of particular platform APIs.

--
nosy: +ned.deily

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 4337a0d9955f0855ba38ef30feec3858d304abf0 by Victor Stinner in 
branch 'master':
bpo-11063: Fix _uuid module on macOS (#3855)
https://github.com/python/cpython/commit/4337a0d9955f0855ba38ef30feec3858d304abf0


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[STINNER Victor]

STINNER Victor  added the comment:

I proposed PR 3855 to add macOS support to _uuid (and fix the compilation 
error).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +3835
stage: resolved -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Antoine Pitrou]

Antoine Pitrou  added the comment:

pkg-config is a Linux-ism.  But Linux already works fine...

$ uname 
Darwin
$ pkg-config
-bash: pkg-config: command not found

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[STINNER Victor]

STINNER Victor  added the comment:

> Though I don't know how to reuse the find_file() logic in configure...

Maybe we could use pkg-config instead?

haypo@selma$ pkg-config uuid --cflags
-I/usr/include/uuid 
haypo@selma$ pkg-config uuid --libs
-luuid

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Though I don't know how to reuse the find_file() logic in configure...

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Antoine Pitrou]

Antoine Pitrou  added the comment:

> Would it possible to check if uuid_generate_time_safe() is available, maybe 
> in configure?

That's probably possible.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[STINNER Victor]

STINNER Victor  added the comment:

> It's expected if uuid_generate_time_safe() isn't available on your platform.  
> But test_uuid still passes?

I would prefer to avoid compilation errors on a popular platforms like macOS. 
Would it possible to check if uuid_generate_time_safe() is available, maybe in 
configure? Or we can "simply" skip _uuid compilation on macOS?

--
resolution: fixed -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Antoine Pitrou]

Antoine Pitrou  added the comment:

It's expected if uuid_generate_time_safe() isn't available on your platform.  
But test_uuid still passes?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Łukasz Langa]

Łukasz Langa  added the comment:

uuid fails to build for me on master since that change landed:


cpython/Modules/_uuidmodule.c:13:11: error: implicit declaration of function 
'uuid_generate_time_safe' is invalid in C99
  [-Werror,-Wimplicit-function-declaration]
res = uuid_generate_time_safe(out);
  ^
cpython/Modules/_uuidmodule.c:13:11: note: did you mean 
'py_uuid_generate_time_safe'?
cpython/Modules/_uuidmodule.c:8:1: note: 'py_uuid_generate_time_safe' declared 
here
py_uuid_generate_time_safe(void)
^
cpython/Modules/_uuidmodule.c:13:11: warning: this function declaration is not 
a prototype [-Wstrict-prototypes]
res = uuid_generate_time_safe(out);
  ^
1 warning and 1 error generated.


This is on macOS Sierra.

--
nosy: +lukasz.langa

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Crude import benchmark (Ubuntu):

* before:

$ time ./python -c "import uuid"

real0m0.074s
user0m0.056s
sys 0m0.012s

* after:

$ time ./python -c "import uuid"

real0m0.030s
user0m0.028s
sys 0m0.000s

* baseline:

$ time ./python -c pass

real0m0.027s
user0m0.024s
sys 0m0.000s

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[STINNER Victor]

STINNER Victor  added the comment:

I ran two benchmarks on my Fedora 26.

* new = master (commit a106aec2ed6ba171838ca7e6ba43c4e722bbecd1)
* ref = commit 8d59aca4a953b097a9b02b0ecafef840e4ac5855

git co master
./python -m perf timeit -s 'import sys, uuid' "del sys.modules['uuid']; import 
uuid; uuid = None" --inherit=PYTHONPATH -v -o import_new.json
./python -m perf timeit -s 'import uuid; u=uuid.uuid1' "u()" 
--inherit=PYTHONPATH -v -o uuid1_new.json

git co 8d59aca4a953b097a9b02b0ecafef840e4ac5855
./python -m perf timeit -s 'import uuid; u=uuid.uuid1' "u()" 
--inherit=PYTHONPATH -v -o uuid1_ref.json
./python -m perf timeit -s 'import sys, uuid' "del sys.modules['uuid']; import 
uuid; uuid = None" --inherit=PYTHONPATH -v -o import_ref.json

Import:

haypo@selma$ ./python -m perf compare_to import_ref.json import_new.json --table
+---++-+
| Benchmark | import_ref | import_new  |
+===++=+
| timeit| 4.04 ms| 430 us: 9.39x faster (-89%) |
+---++-+

uuid.uuid1():

haypo@selma$ ./python -m perf compare_to uuid1_ref.json uuid1_new.json --table
+---+---+--+
| Benchmark | uuid1_ref | uuid1_new|
+===+===+==+
| timeit| 18.9 us   | 15.2 us: 1.24x faster (-20%) |
+---+---+--+

Everything is faster. The import time is 9.4x faster, nice!

In practice, the import time is probably even better. My benchmark uses 
repeated import, it doesn't measure the "first time" import which was more 
expensive because of the "import ctypes".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Antoine Pitrou]

Change by Antoine Pitrou :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[Antoine Pitrou]

Antoine Pitrou  added the comment:


New changeset a106aec2ed6ba171838ca7e6ba43c4e722bbecd1 by Antoine Pitrou in 
branch 'master':
bpo-11063, bpo-20519: avoid ctypes and improve import time for uuid (#3796)
https://github.com/python/cpython/commit/a106aec2ed6ba171838ca7e6ba43c4e722bbecd1


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[STINNER Victor]

STINNER Victor  added the comment:

I abandonned my PR and started to review Antoine's PR 3796 which basically 
combines all previous patches proposed last 6 years :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +3784

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue11063] Rework uuid module: lazy initialization and add a new C extension

[STINNER Victor]

Change by STINNER Victor :


--
title: uuid.py module import has heavy side effects -> Rework uuid module: lazy 
initialization and add a new C extension

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com