Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Patches for thread-support in built-in SHA modules #49071

Closed
ebfe mannequin opened this issue Jan 3, 2009 · 7 comments
Closed

Patches for thread-support in built-in SHA modules #49071

ebfe mannequin opened this issue Jan 3, 2009 · 7 comments
Labels
extension-modules C modules in the Modules dir pending The issue will be closed if no feedback is provided performance Performance or resource usage

Comments

@ebfe
Copy link
Mannequin

ebfe mannequin commented Jan 3, 2009

BPO 4821
Nosy @gpshead, @jcea, @vstinner, @tiran
Files
  • sha1module_small_locks.diff
  • sha256module_small_locks.diff
  • sha512module_small_locks.diff
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = None
created_at = <Date 2009-01-03.17:08:23.573>
labels = ['extension-modules', 'performance']
title = 'Patches for thread-support in built-in SHA modules'
updated_at = <Date 2016-06-12.11:22:32.053>
user = 'https://bugs.python.org/ebfe'

    bugs.python.org fields:

    activity = <Date 2016-06-12.11:22:32.053>
actor = 'christian.heimes'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules']
creation = <Date 2009-01-03.17:08:23.573>
creator = 'ebfe'
dependencies = []
files = ['12569', '12570', '12571']
hgrepos = []
issue_num = 4821
keywords = ['patch']
message_count = 6.0
messages = ['78975', '79146', '79148', '79995', '81728', '172069']
nosy_count = 5.0
nosy_names = ['gregory.p.smith', 'jcea', 'vstinner', 'christian.heimes', 'ebfe']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue4821'
versions = ['Python 3.6']
    @ebfe
    Copy link
    Mannequin Author

    ebfe mannequin commented Jan 3, 2009

    Here is the follow-up to issue bpo-4818. The patches attached allow the
    built-in SHA modules to release the GIL.

    Also the build-in SHA modules will now no longer accept "s#" as input.
    Input is parsed just as in the openssl-driven classes where
    unicode-objects are explicitly rejected.

    The built-in hash modules have been not quite beautiful before even more
    code is now copy & pasted between them. Is there any interest in
    refactoring all those modules? AFAIK _sha1 and such are only used by
    hashlib.py ...

    @vstinner
    Copy link
    Member

    vstinner commented Jan 5, 2009

    This patch is a duplicate of bpo-3745.

    @vstinner
    Copy link
    Member

    vstinner commented Jan 5, 2009

    Oops, it's not a duplicate /o\ But it may solves bpo-3745 (reject unicode
    in sha256).

    @vstinner
    Copy link
    Member

    sha1module_small_locks.diff patch is very similar to the changes made
    in bpo-4751, except:

    • SHA1_GIL_MINSIZE is 8192 whereas HASHLIB_GIL_MINSIZE is 2048
    • There is no test for PyThread_allocate_lock() failure

    Instead of copy/paste code in hashlib, sha1, sha256 and sha512 (4
    modules), can't we share some constants, functions or macros?
    Examples:

    • the GIL minimum size constant
    • the long MY_GET_BUFFER_VIEW_OR_ERROUT macro (which can be a
      function)

    And about sha, why using 3 files for sha? Are the source code so
    different? In the GNU libc, they use "template" files (it's possible
    even with the C language using the preprocessor!): strtof(), strtod()
    and strtold() share 99% of the source code. Interesting content of
    strtof.c :
    ----

    #define	FLOAT		float
#define	FLT		FLT
#ifdef USE_WIDE_CHAR
#define STRTOF		wcstof
#define STRTOF_L	__wcstof_l
#else
# define STRTOF		strtof
# define STRTOF_L	__strtof_l
#endif

#include "strtod.c"

    Refactoring to share code between hash modules will ease the changes,
    eg. release the GIL ;-)

    @gpshead
    Copy link
    Member

    gpshead commented Feb 12, 2009

    fyi - I took care of the unicode data acceptance issue for all hashlib
    related modules in the py3k branch in r69524.

    Yes, the hashlib modules have a -lot- of code in common. Refactoring
    that would be nice but i haven't looked into it. I at least moved the
    commonly used #define into a new hashlib.h for the above change.

    @tiran
    Copy link
    Member

    tiran commented Oct 5, 2012

    I'll integrate your patch once I'm done with my SHA-3 patch bpo-16113. I'm using parts of your patch in my new sha3 code to release the GIL.

    I'll also check if I can share more code between the SHA family modules.

    @tiran tiran added the extension-modules C modules in the Modules dir label Oct 5, 2012
    @tiran tiran self-assigned this Oct 5, 2012
    @tiran tiran added the performance Performance or resource usage label Oct 5, 2012
    @tiran tiran removed their assignment Jun 12, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel
    Copy link
    Member

    @tiran this issue has been idle for a decade. Is it still useful?

    @iritkatriel iritkatriel added the pending The issue will be closed if no feedback is provided label Sep 10, 2022
    @iritkatriel iritkatriel closed this as not planned Won't fix, can't repro, duplicate, stale Oct 7, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    extension-modules C modules in the Modules dir pending The issue will be closed if no feedback is provided performance Performance or resource usage
    Projects
    Threading issues 🧵
    Status: Done
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @gpshead @vstinner @tiran @iritkatriel