Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-99108: Add HACL* Blake2 implementation to hashlib#119316

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.

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

Already on GitHub?Sign in to your account

Merged
gpshead merged 64 commits intopython:mainfrommsprotz:hacl_blake2
Aug 13, 2024

Conversation

msprotz
Copy link
Contributor

@msprotzmsprotz commentedMay 21, 2024•
edited by bedevere-appbot
Loading

This replaces the existing hashlib module with a single implementation that uses HACL*'s Blake2b/Blake2s implementations. We added support for all the modes exposed by the Python API, including tree hashing, leaf nodes, and so on. We ported and merged all of these changes upstream in HACL*, added test vectors based on Python's existing implementation, and exposed everything needed for hashlib.

This is a preliminary PR but I wanted to gather early feedback from@gpshead.

What remains to be done on this PR:

  • add support for HACL* 128-bit/256-bit vectorized implementations
  • benchmark on e.g. a recent Apple ARM to measure perf improvement relative to libb2 (which only has Intel AVX2 support)
  • make a call as to whether I should restore libb2 support or whether Python would be happy with simply bundling HACL*'s vectorized implementations and drop support for building against libb2

This is joint work with@R1kM

gpshead reacted with heart emoji
@ghost
Copy link

ghost commentedMay 21, 2024•
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@msprotz
Copy link
ContributorAuthor

@gpshead if you can trigger a round of CI that would be most helpful. Thanks!

@msprotz
Copy link
ContributorAuthor

So I computed some benchmarks on a variety of machines, comparing HACL's performance with libb2. This is using the benchmarking infrastructure from HACL-packages:https://github.com/cryspen/hacl-packages/blob/main/benchmarks/blake.cc

To read these benchmarks, bear in mind that:

  • libb2 automatically picks the "best" implementation (via CPUID)
  • libb2 has portable C code (which is what gets picked on ARM), and AVX2 code (which is what gets picked for the benchmarks below on Intel machines)
  • HACL* never does runtime CPU detection
  • HACL's Blake2s comes in regular (a.k.a. "32") version, and a 128-bit version (that runs on either NEON or AVX)
  • HACL's Blake2b comes in regular (a.k.a. "32") version, and a 256-bit version (that runs on AVX2)
  • we benchmark various input sizes

M3 Max (ARM)

HACL_blake2b_32_oneshot/0                   121 ns          120 ns      5839173HACL_blake2b_32_oneshot/16                  121 ns          120 ns      5758236HACL_blake2b_32_oneshot/256                 226 ns          225 ns      3098757HACL_blake2b_32_oneshot/4096               3413 ns         3396 ns       206182HACL_blake2b_32_oneshot/65536             54123 ns        53818 ns        12987HACL_blake2b_32_oneshot/1048576          867868 ns       863094 ns          815HACL_blake2b_32_oneshot/16777216       13993602 ns     13929400 ns           50libb2_blake2b_oneshot/0                     148 ns          147 ns      4757374libb2_blake2b_oneshot/16                    146 ns          145 ns      4812849libb2_blake2b_oneshot/256                   276 ns          275 ns      2551783libb2_blake2b_oneshot/4096                 4224 ns         4202 ns       167284libb2_blake2b_oneshot/65536               67478 ns        67179 ns        10367libb2_blake2b_oneshot/1048576           1079958 ns      1073158 ns          644libb2_blake2b_oneshot/16777216         17322373 ns     17220756 ns           41HACL_blake2s_32_oneshot/0                   101 ns          100 ns      6901311HACL_blake2s_32_oneshot/16                  101 ns          101 ns      6980107HACL_blake2s_32_oneshot/256                 370 ns          368 ns      1894196HACL_blake2s_32_oneshot/4096               5771 ns         5740 ns       121353HACL_blake2s_32_oneshot/65536             91417 ns        90835 ns         7762HACL_blake2s_32_oneshot/1048576         1478896 ns      1469956 ns          475HACL_blake2s_32_oneshot/16777216       23471740 ns     23337633 ns           30HACL_blake2s_vec128_oneshot/0               177 ns          176 ns      4077567HACL_blake2s_vec128_oneshot/16              186 ns          185 ns      3864926HACL_blake2s_vec128_oneshot/256             685 ns          682 ns      1044433HACL_blake2s_vec128_oneshot/4096          10868 ns        10819 ns        65872HACL_blake2s_vec128_oneshot/65536        174311 ns       173302 ns         4041HACL_blake2s_vec128_oneshot/1048576     2795034 ns      2779949 ns          255HACL_blake2s_vec128_oneshot/16777216   44968086 ns     44750562 ns           16libb2_blake2s_oneshot/0                     177 ns          175 ns      3965174libb2_blake2s_oneshot/16                    176 ns          175 ns      3977815libb2_blake2s_oneshot/256                   689 ns          684 ns      1034218libb2_blake2s_oneshot/4096                10729 ns        10686 ns        65111libb2_blake2s_oneshot/65536              171900 ns       170899 ns         4119libb2_blake2s_oneshot/1048576           2762123 ns      2746686 ns          255libb2_blake2s_oneshot/16777216         44150450 ns     43912125 ns           16

interpretation:

Intel AVX2 (8-Core Intel Core i9, Macbook Pro, 2019)

HACL_blake2b_32_oneshot/0                   189 ns          188 ns      3681808HACL_blake2b_32_oneshot/16                  192 ns          192 ns      3721267HACL_blake2b_32_oneshot/256                 334 ns          334 ns      2107862HACL_blake2b_32_oneshot/4096               4774 ns         4772 ns       151013HACL_blake2b_32_oneshot/65536             76518 ns        76485 ns         9405HACL_blake2b_32_oneshot/1048576         1180256 ns      1180031 ns          609HACL_blake2b_32_oneshot/16777216       18999771 ns     18996081 ns           37HACL_blake2b_vec256_oneshot/0               144 ns          144 ns      5012424HACL_blake2b_vec256_oneshot/16              142 ns          142 ns      4864185HACL_blake2b_vec256_oneshot/256             247 ns          246 ns      2801502HACL_blake2b_vec256_oneshot/4096           3457 ns         3456 ns       202513HACL_blake2b_vec256_oneshot/65536         54684 ns        54660 ns        12402HACL_blake2b_vec256_oneshot/1048576      878910 ns       878574 ns          793HACL_blake2b_vec256_oneshot/16777216   14755193 ns     14751612 ns           49libb2_blake2b_oneshot/0                     162 ns          162 ns      4324084libb2_blake2b_oneshot/16                    175 ns          175 ns      4303296libb2_blake2b_oneshot/256                   282 ns          282 ns      2425427libb2_blake2b_oneshot/4096                 4318 ns         4313 ns       176491libb2_blake2b_oneshot/65536               63438 ns        63411 ns        10497libb2_blake2b_oneshot/1048576            987715 ns       987456 ns          708libb2_blake2b_oneshot/16777216         16262424 ns     16262024 ns           41HACL_blake2s_32_oneshot/0                   137 ns          136 ns      5224504HACL_blake2s_32_oneshot/16                  133 ns          133 ns      5199706HACL_blake2s_32_oneshot/256                 447 ns          446 ns      1590439HACL_blake2s_32_oneshot/4096               6792 ns         6789 ns       104370HACL_blake2s_32_oneshot/65536            103446 ns       103393 ns         6687HACL_blake2s_32_oneshot/1048576         1770925 ns      1769228 ns          403HACL_blake2s_32_oneshot/16777216       27090139 ns     27082875 ns           24HACL_blake2s_vec128_oneshot/0               121 ns          121 ns      5760132HACL_blake2s_vec128_oneshot/16              127 ns          127 ns      5784359HACL_blake2s_vec128_oneshot/256             378 ns          378 ns      1834978HACL_blake2s_vec128_oneshot/4096           5727 ns         5725 ns       119742HACL_blake2s_vec128_oneshot/65536         88395 ns        88384 ns         7822HACL_blake2s_vec128_oneshot/1048576     1462898 ns      1462569 ns          490HACL_blake2s_vec128_oneshot/16777216   24048063 ns     24040862 ns           29libb2_blake2s_oneshot/0                     102 ns          102 ns      6673913libb2_blake2s_oneshot/16                    105 ns          105 ns      6419663libb2_blake2s_oneshot/256                   367 ns          367 ns      1891554libb2_blake2s_oneshot/4096                 5540 ns         5537 ns       121671libb2_blake2s_oneshot/65536               88250 ns        88211 ns         7868libb2_blake2s_oneshot/1048576           1326051 ns      1325750 ns          519libb2_blake2s_oneshot/16777216         22597565 ns     22584281 ns           32

interpretation:

  • for Blake2b, HACL*/AVX2 10% faster than libb2, HACL*/portable C 16% slower than libb2
  • for Blake2s, HACL*/AVX2 10% slower than libb2, HACL*/portable C 20% slower than libb2 (this is surprising, I'm seeing equal performance using another API, so this may simply be some missingstatic inlines on hot paths)

AVX2 desktop machine, Haswell

HACL_blake2b_32_oneshot/0                   211 ns          211 ns      3096750HACL_blake2b_32_oneshot/16                  212 ns          212 ns      3300382HACL_blake2b_32_oneshot/256                 388 ns          388 ns      1803603HACL_blake2b_32_oneshot/4096               5638 ns         5638 ns       124313HACL_blake2b_32_oneshot/65536             89905 ns        89903 ns         7774HACL_blake2b_32_oneshot/1048576         1438161 ns      1438056 ns          488HACL_blake2b_32_oneshot/16777216       22998077 ns     22997469 ns           30HACL_blake2b_vec256_oneshot/0               168 ns          168 ns      4145151HACL_blake2b_vec256_oneshot/16              168 ns          168 ns      4172085HACL_blake2b_vec256_oneshot/256             309 ns          309 ns      2263628HACL_blake2b_vec256_oneshot/4096           4507 ns         4507 ns       155137HACL_blake2b_vec256_oneshot/65536         72069 ns        72068 ns         9748HACL_blake2b_vec256_oneshot/1048576     1149298 ns      1149303 ns          610HACL_blake2b_vec256_oneshot/16777216   18438313 ns     18436309 ns           38libb2_blake2b_oneshot/0                     190 ns          190 ns      3670823libb2_blake2b_oneshot/16                    194 ns          194 ns      3618244libb2_blake2b_oneshot/256                   353 ns          353 ns      1984245libb2_blake2b_oneshot/4096                 5225 ns         5225 ns       134636libb2_blake2b_oneshot/65536               82645 ns        82642 ns         8502libb2_blake2b_oneshot/1048576           1328695 ns      1328701 ns          529libb2_blake2b_oneshot/16777216         21131069 ns     21130208 ns           33HACL_blake2s_32_oneshot/0                   172 ns          172 ns      4080561HACL_blake2s_32_oneshot/16                  174 ns          174 ns      4024023HACL_blake2s_32_oneshot/256                 605 ns          605 ns      1154699HACL_blake2s_32_oneshot/4096               9210 ns         9210 ns        76047HACL_blake2s_32_oneshot/65536            147054 ns       147054 ns         4744HACL_blake2s_32_oneshot/1048576         2354038 ns      2354016 ns          296HACL_blake2s_32_oneshot/16777216       37686817 ns     37686992 ns           19HACL_blake2s_vec128_oneshot/0               150 ns          150 ns      4686347HACL_blake2s_vec128_oneshot/16              151 ns          151 ns      4628570HACL_blake2s_vec128_oneshot/256             513 ns          513 ns      1367690HACL_blake2s_vec128_oneshot/4096           7748 ns         7748 ns        90207HACL_blake2s_vec128_oneshot/65536        123277 ns       123276 ns         5676HACL_blake2s_vec128_oneshot/1048576     1971694 ns      1971703 ns          355HACL_blake2s_vec128_oneshot/16777216   31514541 ns     31512743 ns           22libb2_blake2s_oneshot/0                     151 ns          151 ns      4651126libb2_blake2s_oneshot/16                    154 ns          154 ns      4542519libb2_blake2s_oneshot/256                   542 ns          542 ns      1293258libb2_blake2s_oneshot/4096                 8381 ns         8381 ns        83415libb2_blake2s_oneshot/65536              133872 ns       133869 ns         5217libb2_blake2s_oneshot/1048576           2140551 ns      2140560 ns          327libb2_blake2s_oneshot/16777216         34272522 ns     34271712 ns           20

interpretation:

  • HACL*/AVX2 13% faster than libb2, HACL*/portable C 8% slower than libb2 (Blake2b)
  • HACL*/AVX2 8% faster than libb2, HACL*/portable C 19% slower than libb2 (Blake2s)

This leaves us with several options. I would love to have your opinion@gpshead

  1. CPython loses the ability to build against libb2, and simply packages HACL's portable versions, which offer a significant performance boost on ARM but are slightly slower on Intel.
  • Pros: CPython build is simplified, drops an external dependency, compelling story with simple C code everywhere
  • Cons: slight performance impact on Intel
  1. CPython loses the ability to build against libb2, and packages HACL's portable versionsand vectorized (128-bit and 256-bit) versions, to be enabled on Intel and AVX (but not on NEON, see issue with high-latency vector shift, above)
  • Pros: CPython increases Blake2 performance across the board (Inteland ARM), + build simplification and loss of an extra dependency
  • Cons: I need to author a CPU detection layer and fiddle with the CPython build (not a big deal, happy to do so)
  1. CPython maintains HACL and libb2 side by side
  • Pros: none
  • Cons: duplication of code, build nightmare, and need to deal with two different APIs for the bindings from C to the Python module.

Please share thoughts. Thanks!

CC@R1kM who provided considerable help with libb2 benchmarking

@gpshead
Copy link
Member

I'd personally go for "2. CPython loses the ability to build against libb2, and packages HACL's portable versions and vectorized".

But even if we only get to option 1. without the cpu detection and just using the portable version, it still seems reasonable. The differences are not huge. And moving from there to option 2 would still be a later followup possibility.

Both of those reduce our dependencies.

@msprotz
Copy link
ContributorAuthor

Happy to do option 2, however, could you help me out a little bit with the build? I've pushed the new files to my branch, but I need to encode in the build system that:

  • Hacl_Hash_Blake2s_Simd128.c should be built with -mavx, and only if the toolchain supports it (as an approximation, this criterion should be "toolchain is Intel") -- otherwise, just don't build it
  • Hacl_Hash_Blake2b_Simd256.c should be built with -mavx2, and only if the toolchain supports it (same) -- otherwise, just don't build it

I don't know if Setup.stdlib.in supports this, and if so, how to encode it. Any pointers appreciated. Thanks!

@eli-schwartz
Copy link
Contributor

# * @MODULE_{NAME}_TRUE@ is removed when configure detects all build
# dependencies for a module. Otherwise the template variable is replaced
# by a comment "#" and the module is skipped.

This is a generalization of a simpler fact: Setup.stdlib.in is processed as an AC_CONFIG_FILES, which means you can do any@SOMETHING@ replacement you could do in the Makefile.

case$argin
-framework)libs="$libs$arg"; skip=libs;
# OSX/OSXS/Darwin framework link cmd
;;
-[IDUCfF]*)cpps="$cpps$arg";;
-Xcompiler)skip=cpps;;
-Xlinker)libs="$libs$arg"; skip=libs;;
-rpath)libs="$libs$arg"; skip=libs;;
--rpath)libs="$libs$arg"; skip=libs;;
-[A-Zl]*)libs="$libs$arg";;

indicates you need to update a match for-m*) before you can use-mavx in a Setup file. On the other hand, you can add arbitrary flagsby reference:

\$\(*_CFLAGS\))cpps="$cpps$arg";;
\$\(*_INCLUDES\))cpps="$cpps$arg";;
\$\(*_LIBS\))libs="$libs$arg";;
\$\(*_LDFLAGS\))libs="$libs$arg";;
\$\(*_RPATH\))libs="$libs$arg";;

so e.g. $(AVX_CFLAGS).

There doesn't currently seem to be a way to build a module where some of its files are built with certain CFLAGS and some are built with other CFLAGS, which indicates you need to pass -mavx* to all files in the blake module, which may be a problem

In commitd777790,@gpshead updated the sha* code from HACL* to build as a standalone static library instead, which then gets linked to in Setup.stdlib.in by specifying a filepath to a*.a file, which makesetup uses to assume it needs to depend on a rule defined in Makefile.pre.in

I think that last approach is what you want to do as that gives you the freedom to define HACL* to build however you need it to, and then Setup.stdlib.in just needs to link to HACL* rather than define the fiddly bits of how to compile it.

Copy link
Contributor

@eli-schwartzeli-schwartz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

At

cpython/Makefile.pre.in

Lines 216 to 219 in769aea3

# Internal static libraries
LIBMPDEC_A= Modules/_decimal/libmpdec/libmpdec.a
LIBEXPAT_A= Modules/expat/libexpat.a
LIBHACL_SHA2_A= Modules/_hacl/libHacl_Hash_SHA2.a

Add:

LIBHACL_BLAKE2_A= Modules/_hacl/libHacl_Hash_BLAKE2.a

After:

cpython/Makefile.pre.in

Lines 638 to 657 in769aea3

##########################################################################
# hashlib's HACL* library
LIBHACL_SHA2_OBJS= \
Modules/_hacl/Hacl_Hash_SHA2.o
LIBHACL_HEADERS= \
Modules/_hacl/include/krml/FStar_UInt128_Verified.h \
Modules/_hacl/include/krml/FStar_UInt_8_16_32_64.h \
Modules/_hacl/include/krml/fstar_uint128_struct_endianness.h \
Modules/_hacl/include/krml/internal/target.h \
Modules/_hacl/include/krml/lowstar_endianness.h \
Modules/_hacl/include/krml/types.h \
Modules/_hacl/Hacl_Streaming_Types.h \
Modules/_hacl/python_hacl_namespaces.h
LIBHACL_SHA2_HEADERS= \
Modules/_hacl/Hacl_Hash_SHA2.h \
Modules/_hacl/internal/Hacl_Hash_SHA2.h \
$(LIBHACL_HEADERS)

Add:

LIBHACL_BLAKE2_AVX_OBJ = Modules/_hacl/Hacl_Hash_Blake2s_Simd128.oLIBHACL_BLAKE2_AVX2_OBJ = Modules/_hacl/Hacl_Hash_Blake2s_Simd256.oLIBHACL_BLAKE2_OBJS=\                Modules/_hacl/Hacl_Hash_Blake2s.o\                Modules/_hacl/Hacl_Hash_Blake2b.o\                @LIBHACL_BLAKE2_AVX_OBJ@\                @LIBHACL_BLAKE2_AVX2_OBJ@\                Modules/_hacl/Lib_Memzero0.o

(and teach configure.ac to replace these@XXX@ with$(XXX) if and only if they are supported by the compiler)

at:

cpython/Makefile.pre.in

Lines 1332 to 1340 in769aea3

# Build HACL* static libraries for hashlib: libHacl_Hash_SHA2.a
LIBHACL_CFLAGS=-I$(srcdir)/Modules/_hacl/include -D_BSD_SOURCE -D_DEFAULT_SOURCE $(PY_STDMODULE_CFLAGS) $(CCSHARED)
Modules/_hacl/Hacl_Hash_SHA2.o: $(srcdir)/Modules/_hacl/Hacl_Hash_SHA2.c $(LIBHACL_SHA2_HEADERS)
$(CC) -c $(LIBHACL_CFLAGS) -o $@ $(srcdir)/Modules/_hacl/Hacl_Hash_SHA2.c
$(LIBHACL_SHA2_A): $(LIBHACL_SHA2_OBJS)
-rm -f $@
$(AR) $(ARFLAGS) $@ $(LIBHACL_SHA2_OBJS)

Add:

LIBHACL_BLAKE2_CFLAGS=-I$(srcdir)/Modules/_hacl/ -I$(srcdir)/Modules/_hacl/include -D_BSD_SOURCE -D_DEFAULT_SOURCEModules/_hacl/Hacl_Hash_Blake2s.o:$(srcdir)/Modules/_hacl/Hacl_Hash_Blake2s.c$(LIBHACL_BLAKE2_HEADERS)$(CC) -c$(LIBHACL_CFLAGS) -o$@$(srcdir)/Modules/_hacl/Hacl_Hash_Blake2s.cModules/_hacl/Hacl_Hash_Blake2b.o:$(srcdir)/Modules/_hacl/Hacl_Hash_Blake2b.c$(LIBHACL_BLAKE2_HEADERS)$(CC) -c$(LIBHACL_CFLAGS) -o$@$(srcdir)/Modules/_hacl/Hacl_Hash_Blake2b.cModules/_hacl/Hacl_Hash_Blake2s_Simd128.o:$(srcdir)/Modules/_hacl/Hacl_Hash_Blake2s_Simd128.c$(LIBHACL_BLAKE2_HEADERS)$(CC) -c$(LIBHACL_CFLAGS) -mavx -o$@$(srcdir)/Modules/_hacl/Hacl_Hash_Blake2s_Simd128.cModules/_hacl/Hacl_Hash_Blake2s_Simd256.o:$(srcdir)/Modules/_hacl/Hacl_Hash_Blake2s_Simd256.c$(LIBHACL_BLAKE2_HEADERS)$(CC) -c$(LIBHACL_CFLAGS) -mavx2 -o$@$(srcdir)/Modules/_hacl/Hacl_Hash_Blake2s_Simd256.cModules/_hacl/Lib_Memzero0.o:$(srcdir)/Modules/_hacl/Lib_Memzero0.c$(LIBHACL_BLAKE2_HEADERS)$(CC) -c$(LIBHACL_CFLAGS) -o$@$(srcdir)/Modules/_hacl/Lib_Memzero0.c$(LIBHACL_BLAKE2_A):$(LIBHACL_BLAKE2_OBJS)-rm -f$@$(AR)$(ARFLAGS)$@$(LIBHACL_BLAKE2_OBJS)

@msprotz
Copy link
ContributorAuthor

Thanks@eli-schwartz that's super helpful. In another issue, someone mentioned that the .a file was causing complications#116043 so I have a pending PR#119320 to remove that. But I have a hunch you might be able to shed some light on the matter :-). Thanks!

@eli-schwartz
Copy link
Contributor

I've never used freeze.py myself. I would guess that it's collecting compiled objects used to produce python and its extensions and thinks it needs the *.a files for that purpose, even though it's used to build extensions, and is simply a matter of it not being adapted as the Makefile evolves.

...

You might think it would be simple to just add$(LIBHACL_BLAKE2_OBJS) to Setup.stdlib.in instead --makesetup supports object files in addition to static archives. Unfortunately it appears thatmakesetup supports it by replacing the .o at the end with a .c, and continuing as though you had originally specified .c files. Then it generates another compile rule.

We really want to use the compile rule from Makefile.pre.in since that allows specifying -mavx per-file...

@msprotz
Copy link
ContributorAuthor

msprotz commentedJul 18, 2024•
edited
Loading

silly question, why expand every single object file into its own recipe instead of relying on a pattern rule?

@eli-schwartz
Copy link
Contributor

silly question, why expand every single object file into its own recipe instead of relying on a pattern rule?

The traditional reason for this is the necessity of supporting Make implementations that don't support the GNU pattern rule extension. Opinions vary about whether to support other Make implementations but the fact of the matter is that you can't even use modern 2024 editions of BSD make in bleeding edge BSD platforms and have those work; you're forced to install the GNU make port and executegmake instead ofmake to build such projects. If depending on GNU specific features it is advised to rename Makefile to GNUmakefile to avoid confusing errors with BSD make and force the use of gmake.

Making such a change requires a bit more discussion than fits this PR, I bet.

@msprotz
Copy link
ContributorAuthor

No worries, that's what I suspected but wanted to double-check. Thanks for your prompt response.

I'm putting the final touches for this, and then you'll have to double-check I haven't used any GNUisms -- all of the Makefiles I've written in recent years assume GNU make > 3.81.

@msprotz
Copy link
ContributorAuthor

Thanks. FreeBSD is good now, and MacOS failures seem unrelated. Do you see anything else suspicious / worth investigating? (I don't.)

@gpshead
Copy link
Member

I don't see any other issues. It won't totally surprise me if some more compiler flag tweaks pop up as necessary in the future on this under various people's configs given those freebsd bots (which should just be using a modern clang or gcc?) didn't like-mavx. they might also not like-mavx2 if run on VMs exposing a modern virtual hardware type but lets wait to find out from the wider world given our buildbots don't show it. I'm merging.

msprotz reacted with thumbs up emoji

@gpsheadgpsheadenabled auto-merge (squash)August 13, 2024 21:15
@gpsheadgpshead merged commit325e9b8 intopython:mainAug 13, 2024
45 checks passed
@msprotzmsprotz deleted the hacl_blake2 branchAugust 13, 2024 21:45
@msprotz
Copy link
ContributorAuthor

Thanks everyone! I'll follow up with the validation / cosmetic changes soon.

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotAMD64 RHEL7 3.x has failed when building commit325e9b8.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#builders/15/builds/8331) and take a look at the build logs.
  4. Check if the failure is related to this commit (325e9b8) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#builders/15/builds/8331

Summary of the results of the build (if available):

Click to see traceback logs
remote:Enumerating objects: 55, done.remote:Counting objects:   2% (1/45)remote:Counting objects:   4% (2/45)remote:Counting objects:   6% (3/45)remote:Counting objects:   8% (4/45)remote:Counting objects:  11% (5/45)remote:Counting objects:  13% (6/45)remote:Counting objects:  15% (7/45)remote:Counting objects:  17% (8/45)remote:Counting objects:  20% (9/45)remote:Counting objects:  22% (10/45)remote:Counting objects:  24% (11/45)remote:Counting objects:  26% (12/45)remote:Counting objects:  28% (13/45)remote:Counting objects:  31% (14/45)remote:Counting objects:  33% (15/45)remote:Counting objects:  35% (16/45)remote:Counting objects:  37% (17/45)remote:Counting objects:  40% (18/45)remote:Counting objects:  42% (19/45)remote:Counting objects:  44% (20/45)remote:Counting objects:  46% (21/45)remote:Counting objects:  48% (22/45)remote:Counting objects:  51% (23/45)remote:Counting objects:  53% (24/45)remote:Counting objects:  55% (25/45)remote:Counting objects:  57% (26/45)remote:Counting objects:  60% (27/45)remote:Counting objects:  62% (28/45)remote:Counting objects:  64% (29/45)remote:Counting objects:  66% (30/45)remote:Counting objects:  68% (31/45)remote:Counting objects:  71% (32/45)remote:Counting objects:  73% (33/45)remote:Counting objects:  75% (34/45)remote:Counting objects:  77% (35/45)remote:Counting objects:  80% (36/45)remote:Counting objects:  82% (37/45)remote:Counting objects:  84% (38/45)remote:Counting objects:  86% (39/45)remote:Counting objects:  88% (40/45)remote:Counting objects:  91% (41/45)remote:Counting objects:  93% (42/45)remote:Counting objects:  95% (43/45)remote:Counting objects:  97% (44/45)remote:Counting objects: 100% (45/45)remote:Counting objects: 100% (45/45), done.remote:Compressing objects:   3% (1/33)remote:Compressing objects:   6% (2/33)remote:Compressing objects:   9% (3/33)remote:Compressing objects:  12% (4/33)remote:Compressing objects:  15% (5/33)remote:Compressing objects:  18% (6/33)remote:Compressing objects:  21% (7/33)remote:Compressing objects:  24% (8/33)remote:Compressing objects:  27% (9/33)remote:Compressing objects:  30% (10/33)remote:Compressing objects:  33% (11/33)remote:Compressing objects:  36% (12/33)remote:Compressing objects:  39% (13/33)remote:Compressing objects:  42% (14/33)remote:Compressing objects:  45% (15/33)remote:Compressing objects:  48% (16/33)remote:Compressing objects:  51% (17/33)remote:Compressing objects:  54% (18/33)remote:Compressing objects:  57% (19/33)remote:Compressing objects:  60% (20/33)remote:Compressing objects:  63% (21/33)remote:Compressing objects:  66% (22/33)remote:Compressing objects:  69% (23/33)remote:Compressing objects:  72% (24/33)remote:Compressing objects:  75% (25/33)remote:Compressing objects:  78% (26/33)remote:Compressing objects:  81% (27/33)remote:Compressing objects:  84% (28/33)remote:Compressing objects:  87% (29/33)remote:Compressing objects:  90% (30/33)remote:Compressing objects:  93% (31/33)remote:Compressing objects:  96% (32/33)remote:Compressing objects: 100% (33/33)remote:Compressing objects: 100% (33/33), done.remote:Total 55 (delta 19), reused 13 (delta 12), pack-reused 10 (from 1)From https://github.com/python/cpython * branch            main       -> FETCH_HEADNote:checking out '325e9b8ef400b86fb077aa40d5cb8cec6e4df7bb'.You are in 'detached HEAD' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by performing another checkout.If you want to create a new branch to retain commits you create, you maydo so (now or later) by using -b with the checkout command again. Example:  git checkout -b new_branch_nameHEAD is now at 325e9b8... gh-99108: Add HACL* Blake2 implementation to hashlib (GH-119316)Switched to and reset branch 'main'configure:WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)../Modules/_hacl/Lib_Memzero0.c:In function ‘Lib_Memzero0_memzero0’:../Modules/_hacl/Lib_Memzero0.c:43:5: error: implicit declaration of function ‘explicit_bzero’ [-Werror=implicit-function-declaration]     explicit_bzero(dst, len_);^~~~~~~~~~~~~~cc1:some warnings being treated as errorsmake:*** [Makefile:1531: Modules/_hacl/Lib_Memzero0.o] Error 1make:*** Waiting for unfinished jobs....find:‘build’: No such file or directoryfind:‘build’: No such file or directoryfind:‘build’: No such file or directoryfind:‘build’: No such file or directorymake:[Makefile:3180: clean-retain-profile] Error 1 (ignored)

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotAMD64 RHEL7 LTO + PGO 3.x has failed when building commit325e9b8.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#builders/96/builds/8086) and take a look at the build logs.
  4. Check if the failure is related to this commit (325e9b8) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#builders/96/builds/8086

Summary of the results of the build (if available):

Click to see traceback logs
remote:Enumerating objects: 55, done.remote:Counting objects:   2% (1/44)remote:Counting objects:   4% (2/44)remote:Counting objects:   6% (3/44)remote:Counting objects:   9% (4/44)remote:Counting objects:  11% (5/44)remote:Counting objects:  13% (6/44)remote:Counting objects:  15% (7/44)remote:Counting objects:  18% (8/44)remote:Counting objects:  20% (9/44)remote:Counting objects:  22% (10/44)remote:Counting objects:  25% (11/44)remote:Counting objects:  27% (12/44)remote:Counting objects:  29% (13/44)remote:Counting objects:  31% (14/44)remote:Counting objects:  34% (15/44)remote:Counting objects:  36% (16/44)remote:Counting objects:  38% (17/44)remote:Counting objects:  40% (18/44)remote:Counting objects:  43% (19/44)remote:Counting objects:  45% (20/44)remote:Counting objects:  47% (21/44)remote:Counting objects:  50% (22/44)remote:Counting objects:  52% (23/44)remote:Counting objects:  54% (24/44)remote:Counting objects:  56% (25/44)remote:Counting objects:  59% (26/44)remote:Counting objects:  61% (27/44)remote:Counting objects:  63% (28/44)remote:Counting objects:  65% (29/44)remote:Counting objects:  68% (30/44)remote:Counting objects:  70% (31/44)remote:Counting objects:  72% (32/44)remote:Counting objects:  75% (33/44)remote:Counting objects:  77% (34/44)remote:Counting objects:  79% (35/44)remote:Counting objects:  81% (36/44)remote:Counting objects:  84% (37/44)remote:Counting objects:  86% (38/44)remote:Counting objects:  88% (39/44)remote:Counting objects:  90% (40/44)remote:Counting objects:  93% (41/44)remote:Counting objects:  95% (42/44)remote:Counting objects:  97% (43/44)remote:Counting objects: 100% (44/44)remote:Counting objects: 100% (44/44), done.remote:Compressing objects:   3% (1/33)remote:Compressing objects:   6% (2/33)remote:Compressing objects:   9% (3/33)remote:Compressing objects:  12% (4/33)remote:Compressing objects:  15% (5/33)remote:Compressing objects:  18% (6/33)remote:Compressing objects:  21% (7/33)remote:Compressing objects:  24% (8/33)remote:Compressing objects:  27% (9/33)remote:Compressing objects:  30% (10/33)remote:Compressing objects:  33% (11/33)remote:Compressing objects:  36% (12/33)remote:Compressing objects:  39% (13/33)remote:Compressing objects:  42% (14/33)remote:Compressing objects:  45% (15/33)remote:Compressing objects:  48% (16/33)remote:Compressing objects:  51% (17/33)remote:Compressing objects:  54% (18/33)remote:Compressing objects:  57% (19/33)remote:Compressing objects:  60% (20/33)remote:Compressing objects:  63% (21/33)remote:Compressing objects:  66% (22/33)remote:Compressing objects:  69% (23/33)remote:Compressing objects:  72% (24/33)remote:Compressing objects:  75% (25/33)remote:Compressing objects:  78% (26/33)remote:Compressing objects:  81% (27/33)remote:Compressing objects:  84% (28/33)remote:Compressing objects:  87% (29/33)remote:Compressing objects:  90% (30/33)remote:Compressing objects:  93% (31/33)remote:Compressing objects:  96% (32/33)remote:Compressing objects: 100% (33/33)remote:Compressing objects: 100% (33/33), done.remote:Total 55 (delta 18), reused 12 (delta 11), pack-reused 11 (from 1)From https://github.com/python/cpython * branch            main       -> FETCH_HEADNote:checking out '325e9b8ef400b86fb077aa40d5cb8cec6e4df7bb'.You are in 'detached HEAD' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by performing another checkout.If you want to create a new branch to retain commits you create, you maydo so (now or later) by using -b with the checkout command again. Example:  git checkout -b new_branch_nameHEAD is now at 325e9b8... gh-99108: Add HACL* Blake2 implementation to hashlib (GH-119316)Switched to and reset branch 'main'configure:WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)find:‘build’: No such file or directoryfind:‘build’: No such file or directoryfind:‘build’: No such file or directoryfind:‘build’: No such file or directorymake[2]:[Makefile:3180: clean-retain-profile] Error 1 (ignored)./Modules/_hacl/Lib_Memzero0.c:In function ‘Lib_Memzero0_memzero0’:./Modules/_hacl/Lib_Memzero0.c:43:5: error: implicit declaration of function ‘explicit_bzero’ [-Werror=implicit-function-declaration]     explicit_bzero(dst, len_);^~~~~~~~~~~~~~cc1:some warnings being treated as errorsmake[2]:*** [Makefile:1531: Modules/_hacl/Lib_Memzero0.o] Error 1make[2]:*** Waiting for unfinished jobs....make[1]:*** [Makefile:903: profile-gen-stamp] Error 2make:*** [Makefile:915: profile-run-stamp] Error 2find:‘build’: No such file or directoryfind:‘build’: No such file or directoryfind:‘build’: No such file or directoryfind:‘build’: No such file or directorymake:[Makefile:3180: clean-retain-profile] Error 1 (ignored)

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotAMD64 RHEL7 LTO 3.x has failed when building commit325e9b8.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#builders/507/builds/8024) and take a look at the build logs.
  4. Check if the failure is related to this commit (325e9b8) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#builders/507/builds/8024

Summary of the results of the build (if available):

Click to see traceback logs
remote:Enumerating objects: 55, done.remote:Counting objects:   2% (1/45)remote:Counting objects:   4% (2/45)remote:Counting objects:   6% (3/45)remote:Counting objects:   8% (4/45)remote:Counting objects:  11% (5/45)remote:Counting objects:  13% (6/45)remote:Counting objects:  15% (7/45)remote:Counting objects:  17% (8/45)remote:Counting objects:  20% (9/45)remote:Counting objects:  22% (10/45)remote:Counting objects:  24% (11/45)remote:Counting objects:  26% (12/45)remote:Counting objects:  28% (13/45)remote:Counting objects:  31% (14/45)remote:Counting objects:  33% (15/45)remote:Counting objects:  35% (16/45)remote:Counting objects:  37% (17/45)remote:Counting objects:  40% (18/45)remote:Counting objects:  42% (19/45)remote:Counting objects:  44% (20/45)remote:Counting objects:  46% (21/45)remote:Counting objects:  48% (22/45)remote:Counting objects:  51% (23/45)remote:Counting objects:  53% (24/45)remote:Counting objects:  55% (25/45)remote:Counting objects:  57% (26/45)remote:Counting objects:  60% (27/45)remote:Counting objects:  62% (28/45)remote:Counting objects:  64% (29/45)remote:Counting objects:  66% (30/45)remote:Counting objects:  68% (31/45)remote:Counting objects:  71% (32/45)remote:Counting objects:  73% (33/45)remote:Counting objects:  75% (34/45)remote:Counting objects:  77% (35/45)remote:Counting objects:  80% (36/45)remote:Counting objects:  82% (37/45)remote:Counting objects:  84% (38/45)remote:Counting objects:  86% (39/45)remote:Counting objects:  88% (40/45)remote:Counting objects:  91% (41/45)remote:Counting objects:  93% (42/45)remote:Counting objects:  95% (43/45)remote:Counting objects:  97% (44/45)remote:Counting objects: 100% (45/45)remote:Counting objects: 100% (45/45), done.remote:Compressing objects:   3% (1/33)remote:Compressing objects:   6% (2/33)remote:Compressing objects:   9% (3/33)remote:Compressing objects:  12% (4/33)remote:Compressing objects:  15% (5/33)remote:Compressing objects:  18% (6/33)remote:Compressing objects:  21% (7/33)remote:Compressing objects:  24% (8/33)remote:Compressing objects:  27% (9/33)remote:Compressing objects:  30% (10/33)remote:Compressing objects:  33% (11/33)remote:Compressing objects:  36% (12/33)remote:Compressing objects:  39% (13/33)remote:Compressing objects:  42% (14/33)remote:Compressing objects:  45% (15/33)remote:Compressing objects:  48% (16/33)remote:Compressing objects:  51% (17/33)remote:Compressing objects:  54% (18/33)remote:Compressing objects:  57% (19/33)remote:Compressing objects:  60% (20/33)remote:Compressing objects:  63% (21/33)remote:Compressing objects:  66% (22/33)remote:Compressing objects:  69% (23/33)remote:Compressing objects:  72% (24/33)remote:Compressing objects:  75% (25/33)remote:Compressing objects:  78% (26/33)remote:Compressing objects:  81% (27/33)remote:Compressing objects:  84% (28/33)remote:Compressing objects:  87% (29/33)remote:Compressing objects:  90% (30/33)remote:Compressing objects:  93% (31/33)remote:Compressing objects:  96% (32/33)remote:Compressing objects: 100% (33/33)remote:Compressing objects: 100% (33/33), done.remote:Total 55 (delta 19), reused 13 (delta 12), pack-reused 10 (from 1)From https://github.com/python/cpython * branch            main       -> FETCH_HEADNote:checking out '325e9b8ef400b86fb077aa40d5cb8cec6e4df7bb'.You are in 'detached HEAD' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by performing another checkout.If you want to create a new branch to retain commits you create, you maydo so (now or later) by using -b with the checkout command again. Example:  git checkout -b new_branch_nameHEAD is now at 325e9b8... gh-99108: Add HACL* Blake2 implementation to hashlib (GH-119316)Switched to and reset branch 'main'configure:WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)./Modules/_hacl/Lib_Memzero0.c:In function ‘Lib_Memzero0_memzero0’:./Modules/_hacl/Lib_Memzero0.c:43:5: error: implicit declaration of function ‘explicit_bzero’ [-Werror=implicit-function-declaration]     explicit_bzero(dst, len_);^~~~~~~~~~~~~~cc1:some warnings being treated as errorsmake:*** [Makefile:1531: Modules/_hacl/Lib_Memzero0.o] Error 1make:*** Waiting for unfinished jobs....find:‘build’: No such file or directoryfind:‘build’: No such file or directoryfind:‘build’: No such file or directoryfind:‘build’: No such file or directorymake:[Makefile:3180: clean-retain-profile] Error 1 (ignored)

@msprotz
Copy link
ContributorAuthor

Oh and yes please keep me posted on RHEL7 and whether it matters or not. If yes, I'll do some more autoconf wizardry, though I can't say this is something I look forward to :-D

@mhsmith
Copy link
Member

mhsmith commentedAug 13, 2024•
edited
Loading

This build failure also happened on Android –#121595 (comment). It looks like it would be sufficient to defineLINUX_NO_EXPLICIT_BZERO on that platform. Though I guess it wouldn't be too hard to add a generic autoconf check for whether the function exists.

@picnixz
Copy link
Member

Thanks everyone! I'll follow up with the validation / cosmetic changes soon.

@msprotz Could you ping me when the PR is ready please? (or just bluntly ask for a review from me?) Thank you in advance.

msprotz reacted with thumbs up emoji

@encukou
Copy link
Member

Oh and yes please keep me posted on RHEL7 and whether it matters or not. I'll do some more autoconf wizardry, though I can't say this is something I look forward to :-D

RHEL7 doesn't matter, but if you don't do the icky wizardry patch, you're effectively leaving it to the maintainer of Android (or if we didn't have Android: some niche BSD that'll struggle to update to 3.14 in a few years).

@mhsmith
Copy link
Member

mhsmith commentedAug 14, 2024•
edited
Loading

This is partly my fault for not setting up the Android buildbot soon enough, so I'll submit a fix today – #99108 (comment).

@asottile
Copy link
Contributor

👋 it looks likeModules/Setup was not updated as part of this patch (and so the deadsnakes builds are failing!)

the line looks like this:

Modules/Setup:#_blake2 _blake2/blake2module.c _blake2/blake2b_impl.c _blake2/blake2s_impl.c

I assume they should look similar to the hashing modules below them?

@msprotz
Copy link
ContributorAuthor

What is Modules/Setup used for?

The build of blake2 is more complicated because some files can only be compiled if the toolchain supported them (see this PR, and notably the changes in configure.ac and Makefile.pre.in).

The logic should be pretty self-explanatory so hopefully you'll know how to fix this? Otherwise, happy to explain in more detail what's happening for the build of the new blake2 module. Thanks!

@eli-schwartz
Copy link
Contributor

@msprotz the file Modules/Setup.stdlib.in was updated in this PR. It actually has a comment that says it isn't used by default "yet". :/

I think that Modules/Setup maybe just has to be updated exactly the same way as the former file?

@msprotz
Copy link
ContributorAuthor

Strange! My local testing picked up my changes in Setup.stdlib.in so I'm not sure in what scenario Setup.stdlib.in is ignored in favor of Setup. But it's a small change so happy to submit a followup PR. See#123146

blhsing pushed a commit to blhsing/cpython that referenced this pull requestAug 22, 2024
…119316)This replaces the existing hashlib Blake2 module with a single implementation that uses HACL\*'s Blake2b/Blake2s implementations. We added support for all the modes exposed by the Python API, including tree hashing, leaf nodes, and so on. We ported and merged all of these changes upstream in HACL\*, added test vectors based on Python's existing implementation, and exposed everything needed for hashlib.This was joint work done with@R1kM.See the PR for much discussion and benchmarking details.   TL;DR: On many systems, 8-50% faster (!) than `libb2`, on some systems it appeared 10-20% slower than `libb2`.
@erlend-aasland
Copy link
Contributor

Modules/Setup is not used by CPython, and it never will. Christian's plan was to replaceModules/Setup with the newModules/Setup.*.in files. The*.in files are now used in the CPython build, the unfortunately,Modules/Setup still remains. I'd prefer it removed.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@eli-schwartzeli-schwartzeli-schwartz requested changes

@picnixzpicnixzpicnixz left review comments

@jmyersmsftjmyersmsftjmyersmsft left review comments

@erlend-aaslanderlend-aaslanderlend-aasland left review comments

@gpsheadgpsheadgpshead approved these changes

@sethmlarsonsethmlarsonAwaiting requested review from sethmlarsonsethmlarson is a code owner

@tirantiranAwaiting requested review from tirantiran is a code owner

@corona10corona10Awaiting requested review from corona10corona10 is a code owner

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

Assignees

@gpsheadgpshead

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

11 participants
@msprotz@gpshead@eli-schwartz@sethmlarson@picnixz@bedevere-bot@mhsmith@encukou@asottile@erlend-aasland@jmyersmsft
[8]ページ先頭
©2009-2025 Movatter.jp