[pull] master from openssl:master #299
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The proposed architectural change focuses on improving concurrency and reducing contention within the method store. The fundamental concept involves moving away from a monolithic synchronisation mechanism—specifically, a single read-write lock (rwlock)—that currently guards the entire method store. Instead of this single point of contention, the strategy is to introduce per-shard synchronisation. This means the method store will be partitioned, or sharded, into several independent segments. Each of these segments, or shards, will be protected by its own dedicated read-write lock. The data in the table below was generated by running evp_fetch twenty times per thread. |---------+----------+---------+---------+---------+---------+---------+---+--------+--------+--------+--------+--------| | | Shards (u/sec) | | Improvements % |---------+----------+---------+---------+---------+---------+---------+---+--------+--------+--------+--------+--------| | Threads | Base | 2 | 4 | 8 | 16 | 32 | | 2 | 4 | 8 | 16 | 32 | |---------+----------+---------+---------+---------+---------+---------+---+--------+--------+--------+--------+--------| | 1 | 0.18282 | 0.18497 | 0.18306 | 0.18314 | 0.18485 | 0.18352 | | 1.17 | 0.13 | 0.18 | 1.11 | 0.39 | | 2 | 0.43588 | 0.35560 | 0.34131 | 0.32516 | 0.33948 | 0.35076 | | -18.42 | -21.70 | -25.40 | -22.12 | -19.53 | | 4 | 1.58185 | 1.06459 | 1.06258 | 0.98698 | 0.98700 | 1.06689 | | -32.70 | -32.83 | -37.61 | -37.60 | -32.55 | | 8 | 3.15686 | 1.75061 | 1.67458 | 1.50241 | 1.62453 | 1.74750 | | -44.55 | -46.95 | -52.41 | -48.54 | -44.64 | | 16 | 5.53647 | 2.83137 | 2.58007 | 2.65972 | 2.64882 | 2.82755 | | -48.86 | -53.40 | -51.96 | -52.16 | -48.93 | | 32 | 10.72727 | 4.97483 | 4.43692 | 4.52524 | 4.68358 | 4.84840 | | -53.62 | -58.64 | -57.82 | -56.34 | -54.80 | | 64 | 21.12103 | 9.43241 | 7.79981 | 7.91148 | 8.33305 | 8.34230 | | -55.34 | -63.07 | -62.54 | -60.55 | -60.50 | Perf tests were running on the system: Architecture: x86_64 CPU op-mode(s): 32-bit, 64-bit Address sizes: 46 bits physical, 48 bits virtual Byte Order: Little Endian CPU(s): 96 On-line CPU(s) list: 0-95 Vendor ID: GenuineIntel Model name: Intel(R) Xeon(R) Gold 6248R CPU @ 3.00GHz CPU family: 6 Model: 85 Thread(s) per core: 2 Core(s) per socket: 24 Socket(s): 2 The most performant option is a configuration with 512 cache entries with 4 shards. There are two new defines NUM_SHARDS, and CACHE_SIZE which can be tweaked at will. Signed-off-by: Nikola Pajkovsky <nikolap@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> Reviewed-by: Norbert Pocs <norbertp@openssl.org> (Merged from #29205)
Unless we have reduced the verbosity level, or overridden it with a HARNESS_TIMER setting. This makes it easier to realize when you are making a test that is starting to dominate testing time for developers, and makes it easier for developers to see it. For: openssl/project#1767 Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Saša Nedvědický <sashan@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from #29408)
Clang format removes a space here, which creates a compiler error when building for PAC/BTI. Fix this and Turn off clang format for this line Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #29399)
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> (Merged from #29392)
This allows setting the ML-KEM and ML-DSA output formats. At the same fixing surprising lack of password encryption of PKCS#8 private keys in DER output form in the CLI apps. Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org> (Merged from #29324)
Coverity flagged a use before NULL check error in kdf_snmpkdf_set_ctx_params. In this function the ctx pointer was dereferenced to obtain a OSSL_LIB_CTX pointer, and only after that dereference was ctx checked for being NULL. fix is pretty clear, just move the OSSL_LIB_CTX computation down to a point after ctx is checked for NULL. fixes openssl/project#1765 Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org> Reviewed-by: Norbert Pocs <norbertp@openssl.org> (Merged from #29400)
The key output will be always private. Reported with a proposed fix by Stanislav Fort (Aisle Research). Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> (Merged from #29397)
This is bascially eating my mac, as it now runs for 80 seconds and eats all the CPU's exercising lock contention. This dials it back to consume at most a quarter of the CPU's in use by HARNESS_JOBS, unless LHASH_WORKERS is set to override it in which case we use that. Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #29406)
For some reason, source reformatting didn't catch everything. Reviewed-by: Paul Yang <paulyang.inf@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from #29383)
This resulted in an adjustment of reformatting that is a bit less trippy in some places. Reviewed-by: Paul Yang <paulyang.inf@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from #29383)
I looked for them with the following grep pipe, and then read through
the output to discern what are actual type definitions, and converted
what I found manually.
git grep -En -e '#define [a-zA-Z_0-9]+ ' -- '*.h' \
| sed -e 's|//.*||' -e 's|/\*.*\*/||g' \
| grep -Ev ' ([0-9]|.*".*|.*\\$)'
Reviewed-by: Paul Yang <paulyang.inf@gmail.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from #29383)
This was done by calling a script I made on the spot:
bash add-types-to-clang-format > .clang-format.new && mv .clang-format.new .clang-format
This is what that script looked like:
#! /bin/bash
sed -ne '1,/^CommentPragmas:/p' < .clang-format
cat <<_____
# OpenSSL uses typedefs extensively. Tell clang-format about them.
TypeNames:
_____
include_re=$(yq -r '.repos.[].hooks.[] | select(.id == "clang-format") | .files' < .pre-commit-config.yaml)
include_dirs=( apps crypto include providers ssl )
git ls-files "${include_dirs[@]}" \
| grep -E $include_re \
| xargs ctags -f - \
| awk -F "\t" -- '$2 ~ /\.h/ && $4 == "t" { printf " - \"%s\"\n", $1 }' \
| sort \
| uniq
cat <<_____
# The following types are macros, and need to remain that way, unfortunately
- "HASH_CTX"
- "HASH_LONG"
- "MD32_REG_T"
_____
sed -ne '/^# OpenSSL uses macros extensively/,$p' < .clang-format
Reviewed-by: Paul Yang <paulyang.inf@gmail.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from #29383)
This resulted in some source reformatting becoming a bit less trippy
This is the script I used to run clang-format appropriately enough:
#! /bin/bash
include_re=$(yq -r '.repos.[].hooks.[] | select(.id == "clang-format") | .files' < .pre-commit-config.yaml)
known_generated=( crypto/bn/bn_prime.h
crypto/objects/obj_dat.h
crypto/objects/obj_xref.h
include/openssl/obj_mac.h
crypto/conf/conf_def.h
crypto/asn1/charmap.h
'*_err.c'
'*err.h' )
exclusions=()
for g in "${known_generated[@]}"; do
# Convert pattern to a regex
g=${g//\./\\.}
g=${g//\*/.\*}
exclusions+=( "^${g}\$" )
done
# Join all exclusions into one regex
exclude_re=$(IFS='|'; echo "${exclusions[*]}")
(set -x; git ls-files | grep -E $include_re | grep -Ev $exclude_re | xargs clang-format -i)
Reviewed-by: Paul Yang <paulyang.inf@gmail.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
(Merged from #29383)
Reviewed-by: Paul Yang <paulyang.inf@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from #29383)
Reviewed-by: Paul Yang <paulyang.inf@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> (Merged from #29383)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )