[issue16496] Simplify and optimize random_seed()

Changes by Mark Dickinson : -- resolution: -> fixed status: open -> closed ___ Python tracker ___ ___ Python-bugs-list mailing list U

[issue16496] Simplify and optimize random_seed()

Roundup Robot added the comment: New changeset db75553ff333 by Mark Dickinson in branch 'default': Simplify random_seed to use _PyLong_AsByteArray. Closes issue #16496. http://hg.python.org/cpython/rev/db75553ff333 -- nosy: +python-dev ___ Python tra

[issue16496] Simplify and optimize random_seed()

Changes by Andrew Svetlov : -- nosy: +asvetlov ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.pyth

[issue16496] Simplify and optimize random_seed()

Changes by Jesús Cea Avión : -- nosy: +jcea ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.

[issue16496] Simplify and optimize random_seed()

Mark Dickinson added the comment: Updated patch: adds in the missing checks for PyMem_Malloc errors. -- Added file: http://bugs.python.org/file28107/random_seed_metd2.patch ___ Python tracker __

[issue16496] Simplify and optimize random_seed()

Changes by Mark Dickinson : -- assignee: -> mark.dickinson ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: htt

[issue16496] Simplify and optimize random_seed()

Stefan Krah added the comment: Is PY_UINT32_T a big problem? I hope that one day we can use the C99 types directly. Visual Studio finally supports stdint.h, and I'm not aware of any compiler that does not. Consider cdecimal as a trial balloon: It compiles on all obscure snakebite platforms, and

[issue16496] Simplify and optimize random_seed()

Serhiy Storchaka added the comment: > I tend to favour direct, readable, and portable code over unnecessarily > optimized code. And my feeling of directness, readability, and portability also slightly differs. I agree that code size and additional operations not of importance here. I say on

[issue16496] Simplify and optimize random_seed()

Antoine Pitrou added the comment: I agree with Mark, we don't need to micro-optimize here. Also, +1 for not needing PY_UINT32_T. -- nosy: +pitrou ___ Python tracker ___

[issue16496] Simplify and optimize random_seed()

Mark Dickinson added the comment: > Apparently my feeling of comfort is different from your own. ;) Yes: I tend to favour direct, readable, and portable code over unnecessarily optimized code. To address the specific points: > The code is larger. Very slightly. It's (IMO) more readable and

[issue16496] Simplify and optimize random_seed()

Serhiy Storchaka added the comment: The code is larger. There is one additional allocation. CPU tacts wasted for uint32->ulong conversion (and in any case all numbers in the generator are 32-bit). One additional ValeError/OverflowError. Apparently my feeling of comfort is different from yo

[issue16496] Simplify and optimize random_seed()

Mark Dickinson added the comment: > I'm uncomfortable with such patch. Any particular reason? It's direct and straightforward, and eliminates the quadratic behaviour. -- ___ Python tracker __

[issue16496] Simplify and optimize random_seed()

Serhiy Storchaka added the comment: I don't think that the preservation of the signature of the auxiliary private static function is worth it. I'm uncomfortable with such patch. But do as you feel comfortable. -- ___ Python tracker

[issue16496] Simplify and optimize random_seed()

Mark Dickinson added the comment: I'm still uncomfortable with the init_by_array signature changes and the use of PY_UINT32_T. How about something like the attached instead? It keeps the central idea (use _PyLong_NumBits and _PyLong_AsByteArray) but doesn't require any signature changes or s

[issue16496] Simplify and optimize random_seed()

Serhiy Storchaka added the comment: Oops, typo. -- Added file: http://bugs.python.org/file28021/random_seed_3.patch ___ Python tracker ___ ___

[issue16496] Simplify and optimize random_seed()

Changes by Serhiy Storchaka : Removed file: http://bugs.python.org/file28020/random_seed_3.patch ___ Python tracker ___ ___ Python-bugs-list m

[issue16496] Simplify and optimize random_seed()

Serhiy Storchaka added the comment: Patch updated. Now random_seed() is platform-independed for integer arguments. -- Added file: http://bugs.python.org/file28020/random_seed_3.patch ___ Python tracker

[issue16496] Simplify and optimize random_seed()

Mark Dickinson added the comment: Thanks for the updated patch. A couple more comments: - you've got potential overflow when computing keysize from bits, on platforms where sizeof(size_t) > sizeof(unsigned long). - please could you move the check for PY_UINT32_T nearer the top of the file, a

[issue16496] Simplify and optimize random_seed()

Mark Dickinson added the comment: > PyUCS4 also should be 32-bit, therefore Python requires such type. Hmm, okay. I wonder whether PY_UINT32_T should have been used there, to avoid doing the same checks in multiple places. > What I really doubt is that now same integer seed on little-endian a

[issue16496] Simplify and optimize random_seed()

Serhiy Storchaka added the comment: Patch updated to conform with Mark's nitpicks. What I really doubt is that now same integer seed on little-endian and big-endian give different random sequences. Is this important? If yes, I can add bytes-swapping. On other hand, non-integer seed already giv

[issue16496] Simplify and optimize random_seed()

Serhiy Storchaka added the comment: Are there any platforms without 32-bit integers (PY_UINT32_T can be uint32_t, unsigned int or long)? PyUCS4 also should be 32-bit, therefore Python requires such type. -- ___ Python tracker

[issue16496] Simplify and optimize random_seed()

Mark Dickinson added the comment: Patch looks correct and looks good to me, modulo a couple of nitpicks (see Rietveld comments). This seems like a nice cleanup. The patch introduces a new dependence on PY_UINT32_T, which is something we haven't so far used elsewhere in Python beyond the float

[issue16496] Simplify and optimize random_seed()

Martin v. Löwis added the comment: Ah. The cleanup looks good, from a shallow glance. I haven't verified yet that the change is actually correct. -- ___ Python tracker ___ _

[issue16496] Simplify and optimize random_seed()

Serhiy Storchaka added the comment: This is not a problem at all. This just a code cleanup. Removing a code duplicate. Performance boost is a side effect. I confused by an old code comment about _PyLong_AsByteArray(). I don't see any difficulties. -- _

[issue16496] Simplify and optimize random_seed()

Martin v. Löwis added the comment: Why is this a problem? -- nosy: +loewis ___ Python tracker ___ ___ Python-bugs-list mailing list Un

[issue16496] Simplify and optimize random_seed()

New submission from Serhiy Storchaka: Now random_seed() use an ineffective quadratic-time algorithm. It can reuse _PyLong_AsByteArray(), decreasing code size and increasing performance. -- components: Extension Modules files: random_seed.patch keywords: patch messages: 175812 nosy: mar