Skip to content

Commit

Permalink
Merge pull request #32725 from vespa-engine/arnej/make-tracenode-thre…
Browse files Browse the repository at this point in the history
…ad-safe

make TraceNode thread safe
  • Loading branch information
arnej27959 authored Oct 31, 2024
2 parents 893280d + d433ab8 commit a3ab2da
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
*
* @author Steinar Knutsen
* @author bratseth
* @deprecated Not used, will be removed in next major release
*/
@Deprecated(forRemoval = true) // TODO: Remove on Vespa 9
public class ThreadRobustList<T> implements Iterable<T> {

private Object[] items;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@
*
* @author Steinar Knutsen
* @author bratseth
* @since 5.1.15
* @deprecated Not used, will be removed in next major release
*/
@Deprecated(forRemoval = true) // TODO: Remove on Vespa 9
public class ThreadRobustList<T> implements Iterable<T> {

private Object[] items;
Expand Down
20 changes: 9 additions & 11 deletions vespajlib/src/main/java/com/yahoo/yolean/trace/TraceNode.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.yolean.trace;

import com.yahoo.yolean.concurrent.ThreadRobustList;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
Expand All @@ -14,21 +14,17 @@
* may contain a payload of any type, the trace tree can be used to exchange any thread-safe state between producers and
* consumers in different threads, whether or not the shape of the trace tree is relevant to the particular
* information.</p>
* <p>This class uses a {@link ThreadRobustList} for its children. That list allows multiple threads to inspect the
* hierarchy of a <code>TraceNode</code> tree while there are other threads concurrently modifying it, without incurring the
* cost of memory synchronization. The only caveat being that for each <code>TraceNode</code> there can never be more than
* exactly one writer thread. If multiple threads need to mutate a single <code>TraceNode</code>, then the writer threads
* need to synchronize their access on the <code>TraceNode</code>.</p>
* <p>This class uses a synchronized list for its children. To avoid contention, each <code>TraceNode</code> should only
* have one writer thread.
*
* @author Steinar Knutsen
* @author bratseth
* @since 5.1.15
*/
public class TraceNode {

private final Object payload;
private final long timestamp;
private ThreadRobustList<TraceNode> children;
private List<TraceNode> children;
private TraceNode parent;

/**
Expand All @@ -55,8 +51,10 @@ public TraceNode add(TraceNode child) {
throw new IllegalArgumentException("Can not add " + child + " to " + this + "; it is not a root.");
}
child.parent = this;
if (children == null) {
children = new ThreadRobustList<>();
synchronized(this) {
if (children == null) {
children = Collections.synchronizedList(new ArrayList<>());
}
}
children.add(child);
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@
/**
* Check we keep the consistent view when reading and writing in parallell.
*
* @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
* @author Steinar Knutsen
*/
@SuppressWarnings("removal")
public class ThreadRobustListTestCase {

private static class Writer implements Runnable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@
import static org.junit.Assert.fail;

/**
* @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
* @author Steinar Knutsen
*/
@SuppressWarnings("removal")
public class ThreadRobustListTestCase {

private final static int NUM_THREADS = 64;
Expand Down

0 comments on commit a3ab2da

Please sign in to comment.