From 350100a5bb9d9671aca85213b2ec7a70a361b0cd Mon Sep 17 00:00:00 2001 From: Ilya Tokar Date: Thu, 19 Sep 2024 09:01:50 -0700 Subject: [PATCH 1/8] Add BrotliCopyPreloadedSymbols function. Add a single trivial use to avoid complier warning. PiperOrigin-RevId: 676435629 --- c/dec/decode.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/c/dec/decode.c b/c/dec/decode.c index 220c7e8..3ed9fe8 100644 --- a/c/dec/decode.c +++ b/c/dec/decode.c @@ -466,6 +466,53 @@ static BROTLI_INLINE brotli_reg_t ReadPreloadedSymbol(const HuffmanCode* table, return result; } +/* Reads up to limit symbols from br and copies them into ringbuffer, + starting from pos. Caller must ensure that there is enough space + for the write. Returns the amount of symbols actually copied. */ +static BROTLI_INLINE int BrotliCopyPreloadedSymbolsToU8(const HuffmanCode* table, + BrotliBitReader* br, + brotli_reg_t* bits, + brotli_reg_t* value, + uint8_t* ringbuffer, + int pos, + const int limit) { + /* Calculate range where CheckInputAmount is always true. + Start with the number of bytes we can read. */ + int64_t new_lim = br->guard_in - br->next_in; + /* Convert to bits, since sybmols use variable number of bits. */ + new_lim *= 8; + /* At most 15 bits per symbol, so this is safe. */ + new_lim /= 15; + const int kMaximalOverread = 4; + int pos_limit = limit; + int copies = 0; + if ((new_lim - kMaximalOverread) <= limit) { + // Safe cast, since new_lim is already < num_steps + pos_limit = (int)(new_lim - kMaximalOverread); + } + if (pos_limit < 0) { + pos_limit = 0; + } + copies = pos_limit; + pos_limit += pos; + /* Fast path, caller made sure it is safe to write, + we verified that is is safe to read. */ + for (; pos < pos_limit; pos++) { + BROTLI_DCHECK(BrotliCheckInputAmount(br)); + ringbuffer[pos] = (uint8_t)ReadPreloadedSymbol(table, br, bits, value); + BROTLI_LOG_ARRAY_INDEX(ringbuffer, pos); + } + /* Do the remainder, caller made sure it is safe to write, + we need to bverify that it is safe to read. */ + while (BrotliCheckInputAmount(br) && copies < limit) { + ringbuffer[pos] = (uint8_t)ReadPreloadedSymbol(table, br, bits, value); + BROTLI_LOG_ARRAY_INDEX(ringbuffer, pos); + pos++; + copies++; + } + return copies; +} + static BROTLI_INLINE brotli_reg_t Log2Floor(brotli_reg_t x) { brotli_reg_t result = 0; while (x) { @@ -1969,8 +2016,8 @@ CommandInner: goto NextLiteralBlock; } if (!safe) { - s->ringbuffer[pos] = - (uint8_t)ReadPreloadedSymbol(s->literal_htree, br, &bits, &value); + BrotliCopyPreloadedSymbolsToU8(s->literal_htree, br, &bits, &value, + s->ringbuffer, pos, 1); } else { brotli_reg_t literal; if (!SafeReadSymbol(s->literal_htree, br, &literal)) { From 664952333f675eb189b9d8dfefb1d33538cc41f6 Mon Sep 17 00:00:00 2001 From: Ilya Tokar Date: Thu, 24 Oct 2024 13:36:16 -0700 Subject: [PATCH 2/8] Make Brotli decompression faster Makes it ~8% faster on my skylake desktop. PiperOrigin-RevId: 689499172 --- c/dec/decode.c | 75 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 19 deletions(-) diff --git a/c/dec/decode.c b/c/dec/decode.c index 3ed9fe8..921aa4a 100644 --- a/c/dec/decode.c +++ b/c/dec/decode.c @@ -2006,35 +2006,72 @@ CommandInner: brotli_reg_t bits; brotli_reg_t value; PreloadSymbol(safe, s->literal_htree, br, &bits, &value); - do { - if (!CheckInputAmount(safe, br)) { - s->state = BROTLI_STATE_COMMAND_INNER; - result = BROTLI_DECODER_NEEDS_MORE_INPUT; - goto saveStateAndReturn; + if (!safe) { + // This is a hottest part of the decode, so we copy the loop below + // and optimize it by calculating the number of steps where all checks + // evaluate to false (ringbuffer size/block size/input size). + // Since all checks are loop invariant, we just need to find + // minimal number of iterations for a simple loop, and run + // the full version for the remainder. + int num_steps = i - 1; + if (num_steps > 0 && ((brotli_reg_t)(num_steps) > s->block_length[0])) { + // Safe cast, since block_length < steps + num_steps = (int)s->block_length[0]; } - if (BROTLI_PREDICT_FALSE(s->block_length[0] == 0)) { - goto NextLiteralBlock; + if (s->ringbuffer_size >= pos && + (s->ringbuffer_size - pos) <= num_steps) { + num_steps = s->ringbuffer_size - pos - 1; } - if (!safe) { + if (num_steps < 0) { + num_steps = 0; + } + num_steps = BrotliCopyPreloadedSymbolsToU8(s->literal_htree, br, &bits, + &value, s->ringbuffer, pos, + num_steps); + pos += num_steps; + s->block_length[0] -= (brotli_reg_t)num_steps; + i -= num_steps; + do { + if (!CheckInputAmount(safe, br)) { + s->state = BROTLI_STATE_COMMAND_INNER; + result = BROTLI_DECODER_NEEDS_MORE_INPUT; + goto saveStateAndReturn; + } + if (BROTLI_PREDICT_FALSE(s->block_length[0] == 0)) { + goto NextLiteralBlock; + } BrotliCopyPreloadedSymbolsToU8(s->literal_htree, br, &bits, &value, s->ringbuffer, pos, 1); - } else { + --s->block_length[0]; + BROTLI_LOG_ARRAY_INDEX(s->ringbuffer, pos); + ++pos; + if (BROTLI_PREDICT_FALSE(pos == s->ringbuffer_size)) { + s->state = BROTLI_STATE_COMMAND_INNER_WRITE; + --i; + goto saveStateAndReturn; + } + } while (--i != 0); + } else { /* safe */ + do { + if (BROTLI_PREDICT_FALSE(s->block_length[0] == 0)) { + goto NextLiteralBlock; + } brotli_reg_t literal; if (!SafeReadSymbol(s->literal_htree, br, &literal)) { result = BROTLI_DECODER_NEEDS_MORE_INPUT; goto saveStateAndReturn; } s->ringbuffer[pos] = (uint8_t)literal; - } - --s->block_length[0]; - BROTLI_LOG_ARRAY_INDEX(s->ringbuffer, pos); - ++pos; - if (BROTLI_PREDICT_FALSE(pos == s->ringbuffer_size)) { - s->state = BROTLI_STATE_COMMAND_INNER_WRITE; - --i; - goto saveStateAndReturn; - } - } while (--i != 0); + --s->block_length[0]; + BROTLI_LOG_ARRAY_INDEX(s->ringbuffer, pos); + ++pos; + if (BROTLI_PREDICT_FALSE(pos == s->ringbuffer_size)) { + s->state = BROTLI_STATE_COMMAND_INNER_WRITE; + --i; + goto saveStateAndReturn; + } + } while (--i != 0); + } } else { uint8_t p1 = s->ringbuffer[(pos - 1) & s->ringbuffer_mask]; uint8_t p2 = s->ringbuffer[(pos - 2) & s->ringbuffer_mask]; From 4303850b01d6cf299fd532c7cc58bab0e2dce4fb Mon Sep 17 00:00:00 2001 From: Evgenii Kliuchnikov Date: Mon, 11 Nov 2024 07:50:54 -0800 Subject: [PATCH 3/8] No public description PiperOrigin-RevId: 695336284 --- java/org/brotli/dec/build_defs.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/org/brotli/dec/build_defs.bzl b/java/org/brotli/dec/build_defs.bzl index 40ccbd1..e25f40f 100644 --- a/java/org/brotli/dec/build_defs.bzl +++ b/java/org/brotli/dec/build_defs.bzl @@ -1,6 +1,6 @@ """Utilities for Java brotli tests.""" -load("//third_party/bazel_rules/rules_java/java:java_test.bzl", "java_test") +load("@rules_java//java:java_test.bzl", "java_test") _TEST_JVM_FLAGS = [ "-DBROTLI_ENABLE_ASSERTS=true", From 7347e81db52922ab209530ff8dac32512aef6120 Mon Sep 17 00:00:00 2001 From: Evgenii Kliuchnikov Date: Mon, 11 Nov 2024 14:03:12 +0000 Subject: [PATCH 4/8] Fix Java Bazel build --- java/WORKSPACE.bazel | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/java/WORKSPACE.bazel b/java/WORKSPACE.bazel index 0b23445..9e9ef28 100644 --- a/java/WORKSPACE.bazel +++ b/java/WORKSPACE.bazel @@ -7,15 +7,29 @@ local_repository( load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive", "http_file") +RULES_JAVA_VERSION = "8.3.2" +RULES_JAVA_SHA = "9b9614f8a7f7b7ed93cb7975d227ece30fe7daed2c0a76f03a5ee37f69e437de" RULES_JVM_EXTERNAL_TAG = "4.0" RULES_JVM_EXTERNAL_SHA = "31701ad93dbfe544d597dbe62c9a1fdd76d81d8a9150c2bf1ecf928ecdf97169" RULES_KOTLIN_VERSION = "1.9.0" RULES_KOTLIN_SHA = "5766f1e599acf551aa56f49dab9ab9108269b03c557496c54acaf41f98e2b8d6" +http_archive( + name = "rules_java", + sha256 = RULES_JAVA_SHA, + urls = [ + "https://github.com/bazelbuild/rules_java/releases/download/%s/rules_java-%s.tar.gz" % (RULES_JAVA_VERSION, RULES_JAVA_VERSION), + ], +) + +load("@rules_java//java:repositories.bzl", "rules_java_dependencies", "rules_java_toolchains") +rules_java_dependencies() +rules_java_toolchains() + http_archive( name = "rules_jvm_external", - strip_prefix = "rules_jvm_external-%s" % RULES_JVM_EXTERNAL_TAG, sha256 = RULES_JVM_EXTERNAL_SHA, + strip_prefix = "rules_jvm_external-%s" % RULES_JVM_EXTERNAL_TAG, url = "https://github.com/bazelbuild/rules_jvm_external/archive/%s.zip" % RULES_JVM_EXTERNAL_TAG, ) @@ -23,15 +37,15 @@ load("@rules_jvm_external//:defs.bzl", "maven_install") http_archive( name = "rules_kotlin", - urls = ["https://github.com/bazelbuild/rules_kotlin/releases/download/v%s/rules_kotlin-v%s.tar.gz" % (RULES_KOTLIN_VERSION, RULES_KOTLIN_VERSION)], sha256 = RULES_KOTLIN_SHA, + urls = ["https://github.com/bazelbuild/rules_kotlin/releases/download/v%s/rules_kotlin-v%s.tar.gz" % (RULES_KOTLIN_VERSION, RULES_KOTLIN_VERSION)], ) load("@rules_kotlin//kotlin:repositories.bzl", "kotlin_repositories") -kotlin_repositories() # if you want the default. Otherwise see custom kotlinc distribution below +kotlin_repositories() # if you want the default. Otherwise see custom kotlinc distribution below load("@rules_kotlin//kotlin:core.bzl", "kt_register_toolchains") -kt_register_toolchains() # to use the default toolchain, otherwise see toolchains below +kt_register_toolchains() # to use the default toolchain, otherwise see toolchains below maven_install( artifacts = ["junit:junit:4.12"], @@ -43,37 +57,37 @@ maven_install( http_archive( name = "platforms", + sha256 = "8150406605389ececb6da07cbcb509d5637a3ab9a24bc69b1101531367d89d74", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/platforms/releases/download/0.0.8/platforms-0.0.8.tar.gz", "https://github.com/bazelbuild/platforms/releases/download/0.0.8/platforms-0.0.8.tar.gz", ], - sha256 = "8150406605389ececb6da07cbcb509d5637a3ab9a24bc69b1101531367d89d74", ) http_file( name = "openjdk_jni_h", downloaded_file_path = "jni.h", - urls = ["https://raw.githubusercontent.com/openjdk/jdk/jdk8-b120/jdk/src/share/javavm/export/jni.h"], sha256 = "ed99792df48670072b78028faf704a8dcb6868fe140ccc7eced9b01dfa62fef4", + urls = ["https://raw.githubusercontent.com/openjdk/jdk/jdk8-b120/jdk/src/share/javavm/export/jni.h"], ) http_file( name = "openjdk_solaris_jni_md_h", downloaded_file_path = "jni_md.h", - urls = ["https://raw.githubusercontent.com/openjdk/jdk/jdk8-b120/jdk/src/solaris/javavm/export/jni_md.h"], sha256 = "b6cf7b06e5bba38d2daa2ff0789f99d396b3cb3bcc37d0367c8360fdccdef294", + urls = ["https://raw.githubusercontent.com/openjdk/jdk/jdk8-b120/jdk/src/solaris/javavm/export/jni_md.h"], ) http_file( name = "openjdk_macosx_jni_md_h", downloaded_file_path = "jni_md.h", - urls = ["https://raw.githubusercontent.com/openjdk/jdk/jdk8-b120/jdk/src/macosx/javavm/export/jni_md.h"], sha256 = "8f718071022e7e7f2fc9a229984b7e83582db91ed83861b49ce1461436fe8dc4", + urls = ["https://raw.githubusercontent.com/openjdk/jdk/jdk8-b120/jdk/src/macosx/javavm/export/jni_md.h"], ) http_file( name = "openjdk_windows_jni_md_h", downloaded_file_path = "jni_md.h", - urls = ["https://raw.githubusercontent.com/openjdk/jdk/jdk8-b120/jdk/src/windows/javavm/export/jni_md.h"], sha256 = "5479fb385ea1e11619f5c0cdfd9ccb3ea3a3fea0f5bc6176fb3ce62be29d759b", + urls = ["https://raw.githubusercontent.com/openjdk/jdk/jdk8-b120/jdk/src/windows/javavm/export/jni_md.h"], ) From 4c4b1297c60945e21afe3a08511b0a622e659af5 Mon Sep 17 00:00:00 2001 From: Evgenii Kliuchnikov Date: Tue, 12 Nov 2024 12:44:54 +0000 Subject: [PATCH 5/8] "Hermetise" bazel-java tests Should fix windows pipeline. --- .github/workflows/build_test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml index ec13929..532c428 100644 --- a/.github/workflows/build_test.yml +++ b/.github/workflows/build_test.yml @@ -266,7 +266,7 @@ jobs: if: ${{ matrix.build_system == 'bazel' }} run: | cd ${GITHUB_WORKSPACE}/${{ matrix.bazel_project }} - bazelisk build -c opt ...:all + bazelisk build -c opt ...:all --java_runtime_version=remotejdk_21 - name: Fix symlinks for Bazel (Windows) if: ${{ matrix.build_system == 'bazel' && runner.os == 'Windows' && matrix.bazel_project == 'java' }} @@ -311,7 +311,7 @@ jobs: run: | cd ${GITHUB_WORKSPACE}/${{ matrix.bazel_project }} bazelisk query "tests(...)" --output=label > ${RUNNER_TEMP}/tests.lst - [ -s ${RUNNER_TEMP}/tests.lst ] && bazelisk test -c opt ...:all + [ -s ${RUNNER_TEMP}/tests.lst ] && bazelisk test -c opt ...:all --java_runtime_version=remotejdk_21 bazelisk clean - name: Build / Test with Maven From cb63a61918b626c26f11a059417d0cbcbe182563 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:23:33 +0000 Subject: [PATCH 6/8] Bump actions/setup-python from 5.0.0 to 5.3.0 Bumps [actions/setup-python](https://github.com/actions/setup-python) from 5.0.0 to 5.3.0. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](https://github.com/actions/setup-python/compare/0a5c61591373683505ea898e09a3ea4f39ef2b9c...0b93645e9fea7318ecaed2b359559ac225c90a2b) --- updated-dependencies: - dependency-name: actions/setup-python dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- .github/workflows/build_test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml index 532c428..166f6bb 100644 --- a/.github/workflows/build_test.yml +++ b/.github/workflows/build_test.yml @@ -324,7 +324,7 @@ jobs: # cd integration # mvn -B verify - - uses: actions/setup-python@0a5c61591373683505ea898e09a3ea4f39ef2b9c # v5.0.0 + - uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0 if: ${{ matrix.build_system == 'python' }} with: python-version: ${{ matrix.python_version }} From d3471e6ff8f7bc337c25eab1ff227bfbffd228ae Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:23:35 +0000 Subject: [PATCH 7/8] Bump actions/cache from 3.3.2 to 4.1.2 Bumps [actions/cache](https://github.com/actions/cache) from 3.3.2 to 4.1.2. - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](https://github.com/actions/cache/compare/704facf57e6136b1bc63b828d79edcd491f0ee84...6849a6489940f00c2f30c0fb92c6274307ccb58a) --- updated-dependencies: - dependency-name: actions/cache dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/release.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index c99cc12..55ea7e6 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -53,7 +53,7 @@ jobs: submodules: false fetch-depth: 1 - - uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84 # v3.3.2 + - uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2 id: cache-vcpkg with: path: vcpkg From c766253ccce4d9949f4fb026eeb6beecf38409b4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 12 Nov 2024 14:02:26 +0000 Subject: [PATCH 8/8] Bump actions/upload-artifact from 4.0.0 to 4.4.3 Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4.0.0 to 4.4.3. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/c7d193f32edcb7bfad88892161225aeda64e9392...b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- .github/workflows/fuzz.yml | 2 +- .github/workflows/release.yaml | 2 +- .github/workflows/scorecard.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index a1720bf..bebc560 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -28,7 +28,7 @@ jobs: fuzz-seconds: 600 dry-run: false - name: Upload Crash - uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 # v4.0.0 + uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3 if: failure() with: name: artifacts diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 55ea7e6..08b8a0e 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -102,7 +102,7 @@ jobs: cmake --build out --config Release --target install cp LICENSE prefix/bin/LICENSE.brotli - name: Upload artifacts - uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 # v4.0.0 + uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3 with: name: brotli-${{matrix.triplet}} path: | diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index 0c5193f..f79ee6c 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -63,7 +63,7 @@ jobs: # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF # format to the repository Actions tab. - name: "Upload artifact" - uses: actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392 # v4.0.0 + uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3 with: name: SARIF file path: results.sarif