You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by ma...@apache.org on 2023/01/10 17:55:27 UTC
[logging-log4j2] branch master updated: Optimize Lazy::relaxed
This is an automated email from the ASF dual-hosted git repository.
mattsicker pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/master by this push:
new d389efe2ef Optimize Lazy::relaxed
new 0f85bf4f7c Merge pull request #1195 from jvz/dev-master-lazy-relaxed
d389efe2ef is described below
commit d389efe2efb6f4f535cb51b76f47a9b562ef9657
Author: Matt Sicker <ms...@apple.com>
AuthorDate: Mon Jan 9 14:55:57 2023 -0600
Optimize Lazy::relaxed
This fixes the relaxed Lazy<T> implementation to use the witness value from compareAndExchangeRelease as a potential return value rather than doing a second getAcquire to see the existing value. This adds some tests that are in Kotlin for its equivalent lazy implementations.
---
.../org/apache/logging/log4j/util/LazyTest.java | 170 +++++++++++++++++++++
.../java/org/apache/logging/log4j/util/Lazy.java | 7 +-
.../org/apache/logging/log4j/util/LazyUtil.java | 7 +-
3 files changed, 176 insertions(+), 8 deletions(-)
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/LazyTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/LazyTest.java
new file mode 100644
index 0000000000..e25324493e
--- /dev/null
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/LazyTest.java
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache license, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the license for the specific language governing permissions and
+ * limitations under the license.
+ */
+package org.apache.logging.log4j.util;
+
+import java.util.*;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.BiFunction;
+import java.util.function.Predicate;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import org.junit.function.ThrowingRunnable;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+// tests adapted from Kotlin
+@Timeout(30)
+class LazyTest {
+ final List<Throwable> asyncErrors = Collections.synchronizedList(new ArrayList<>());
+ final ThreadGroup threadGroup = new ThreadGroup("LazyTest");
+
+ @AfterEach
+ void tearDown() {
+ threadGroup.interrupt();
+ assertThat(asyncErrors).isEmpty();
+ }
+
+ @Test
+ void strictLazy() {
+ AtomicInteger counter = new AtomicInteger();
+ Lazy<Integer> lazy = Lazy.lazy(() -> {
+ int value = counter.incrementAndGet();
+ // simulate some computation
+ runCatching(() -> Thread.sleep(16));
+ return value;
+ });
+ int threads = 3;
+ CyclicBarrier barrier = new CyclicBarrier(threads);
+ List<Thread> runners = IntStream.range(0, threads)
+ .mapToObj(i -> {
+ Thread thread = new Thread(threadGroup, () -> runCatching(() -> {
+ barrier.await();
+ lazy.get();
+ }));
+ thread.start();
+ return thread;
+ })
+ .collect(Collectors.toList());
+ runners.forEach(thread -> runCatching(thread::join));
+ assertThat(counter.get()).isEqualTo(1);
+ }
+
+ @Test
+ void relaxedLazy() {
+ AtomicInteger counter = new AtomicInteger();
+ int nrThreads = 3;
+ int[] values = ThreadLocalRandom.current()
+ .ints(nrThreads)
+ .map(i -> 100 + i % 50)
+ .toArray();
+ int[] runs = new int[nrThreads];
+ Lazy<Integer> lazy = Lazy.relaxed(() -> {
+ int id = counter.getAndIncrement();
+ int value = values[id];
+ runs[id] = value;
+ runCatching(() -> Thread.sleep(value));
+ return value;
+ });
+ CyclicBarrier barrier = new CyclicBarrier(nrThreads);
+ List<Thread> threads = IntStream.range(0, nrThreads)
+ .mapToObj(i -> {
+ Thread thread = new Thread(threadGroup, () -> runCatching(() -> {
+ barrier.await();
+ lazy.get();
+ }));
+ thread.start();
+ return thread;
+ })
+ .collect(Collectors.toList());
+ while (!lazy.isInitialized()) {
+ runCatching(() -> Thread.sleep(1));
+ }
+ int result = lazy.get();
+ threads.forEach(thread -> runCatching(thread::join));
+ assertThat(counter.get()).isEqualTo(nrThreads);
+ assertThat(lazy.get()).isEqualTo(result);
+ assertThat(runs).contains(result);
+ }
+
+ @Test
+ void strictLazyRace() {
+ racyTest(3, 5000,
+ () -> {
+ AtomicInteger counter = new AtomicInteger();
+ return Lazy.lazy(counter::incrementAndGet);
+ },
+ (lazy, ignored) -> lazy.value(),
+ result -> result.stream().allMatch(i -> i == 1));
+ }
+
+ @Test
+ void relaxedLazyRace() {
+ racyTest(3, 5000,
+ () -> Lazy.relaxed(() -> Thread.currentThread().getId()),
+ (lazy, ignored) -> lazy.value(),
+ result -> result.stream().allMatch(v -> Objects.equals(v, result.get(0))));
+ }
+
+ void runCatching(ThrowingRunnable runnable) {
+ try {
+ runnable.run();
+ } catch (Throwable e) {
+ asyncErrors.add(e);
+ }
+ }
+
+ <S, T> void racyTest(int nrThreads, int runs, Supplier<S> stateInitializer,
+ BiFunction<S, Integer, T> run, Predicate<List<T>> resultsValidator) {
+ List<T> runResult = Collections.synchronizedList(new ArrayList<>());
+ List<Map.Entry<Integer, List<T>>> invalidResults = Collections.synchronizedList(new ArrayList<>());
+ AtomicInteger currentRunId = new AtomicInteger(0);
+ AtomicReference<S> state = new AtomicReference<>();
+ CyclicBarrier barrier = new CyclicBarrier(nrThreads, () -> {
+ int runId = currentRunId.getAndIncrement();
+ if (runId > 0) {
+ if (!resultsValidator.test(runResult)) {
+ invalidResults.add(Map.entry(runId, List.copyOf(runResult)));
+ }
+ runResult.clear();
+ }
+ state.set(stateInitializer.get());
+ });
+ List<Thread> runners = IntStream.range(0, nrThreads)
+ .mapToObj(i -> {
+ Thread thread = new Thread(threadGroup, () -> runCatching(() -> {
+ barrier.await();
+ for (int j = 0; j < runs; j++) {
+ runResult.add(run.apply(state.get(), j));
+ barrier.await();
+ }
+ }));
+ thread.start();
+ return thread;
+ })
+ .collect(Collectors.toList());
+ runners.forEach(thread -> runCatching(thread::join));
+ assertThat(invalidResults).isEmpty();
+ }
+}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/Lazy.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/Lazy.java
index 1c08f6d8d8..09894885ac 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util/Lazy.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/Lazy.java
@@ -42,7 +42,8 @@ public interface Lazy<T> extends Supplier<T> {
void set(final T newValue);
/**
- * Creates a lazy value using the provided Supplier for initialization guarded by a Lock.
+ * Creates a strict lazy value using the provided Supplier. The supplier is guaranteed to only be invoked by at
+ * most one thread, and all threads will see the same published value when this returns.
*/
static <T> Lazy<T> lazy(final Supplier<T> supplier) {
Objects.requireNonNull(supplier);
@@ -57,8 +58,8 @@ public interface Lazy<T> extends Supplier<T> {
}
/**
- * Creates a relaxed lazy value using the provided Supplier for initialization which may be invoked more than once
- * in order to set the initialized value.
+ * Creates a relaxed lazy value using the provided Supplier. The supplier may be invoked by more than one thread,
+ * but all threads will seem the same published value when this returns.
*/
static <T> Lazy<T> relaxed(final Supplier<T> supplier) {
Objects.requireNonNull(supplier);
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/LazyUtil.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/LazyUtil.java
index 0150aaf289..4a3a639b93 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util/LazyUtil.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/LazyUtil.java
@@ -138,11 +138,8 @@ final class LazyUtil {
return unwrapNull(currentValue);
}
final T newValue = supplier.get();
- if (VALUE.compareAndExchangeRelease(this, null, wrapNull(newValue)) == null) {
- return newValue;
- }
- final Object value = VALUE.getAcquire(this);
- return unwrapNull(value);
+ final Object witness = VALUE.compareAndExchangeRelease(this, null, wrapNull(newValue));
+ return witness == null ? newValue : unwrapNull(witness);
}
@Override