Don't lock segment from subsegment (aws#306)

* Don't lock segment from subsegment * synchronized list
willarmiros · Oct 7, 2021 · d5ccae2 · d5ccae2
1 parent 0fe77bb
commit d5ccae2
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 23 deletions.
diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java
@@ -36,6 +36,7 @@
 import com.fasterxml.jackson.databind.ser.std.ToStringSerializer;
 import com.google.errorprone.annotations.concurrent.GuardedBy;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -197,7 +198,7 @@ protected EntityImpl(AWSXRayRecorder creator, String name) {
 
         this.creator = creator;
         this.name = name;
-        this.subsegments = new ArrayList<>();
+        this.subsegments = Collections.synchronizedList(new ArrayList<>());
         this.subsegmentsLock = new ReentrantLock();
         this.cause = new Cause();
         this.http = new HashMap<>();
@@ -531,25 +532,21 @@ public void setParentId(@Nullable String parentId) {
 
     @Override
     public List<Subsegment> getSubsegments() {
-        synchronized (lock) {
-            return subsegments;
-        }
+        return subsegments;
     }
 
     @JsonIgnore
     @Override
     public List<Subsegment> getSubsegmentsCopy() {
-        synchronized (lock) {
-            return new ArrayList<>(subsegments);
-        }
+        return new ArrayList<>(subsegments);
     }
 
     @Override
     public void addSubsegment(Subsegment subsegment) {
         synchronized (lock) {
             checkAlreadyEmitted();
-            subsegments.add(subsegment);
         }
+        subsegments.add(subsegment);
     }
 
     @Override
@@ -675,8 +672,8 @@ public void incrementReferenceCount() {
         synchronized (lock) {
             checkAlreadyEmitted();
             referenceCount++;
-            totalSize.increment();
         }
+        totalSize.increment();
     }
 
     @Override
@@ -706,9 +703,7 @@ public int getReferenceCount() {
      */
     @Override
     public LongAdder getTotalSize() {
-        synchronized (lock) {
-            return totalSize;
-        }
+        return totalSize;
     }
 
     /**
@@ -770,10 +765,8 @@ public String prettySerialize() {
 
     @Override
     public void removeSubsegment(Subsegment subsegment) {
-        synchronized (lock) {
-            subsegments.remove(subsegment);
-            getParentSegment().getTotalSize().decrement();
-        }
+        subsegments.remove(subsegment);
+        getParentSegment().getTotalSize().decrement();
     }
 
 

diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java
@@ -34,8 +34,7 @@ public class SegmentImpl extends EntityImpl implements Segment {
     protected Map<String, Object> service;
 
     @JsonIgnore
-    @GuardedBy("lock")
-    private boolean sampled;
+    private volatile boolean sampled;
 
     @SuppressWarnings({ "unused", "nullness" })
     private SegmentImpl() {
@@ -88,17 +87,15 @@ public boolean isRecording() {
 
     @Override
     public boolean isSampled() {
-        synchronized (lock) {
-            return sampled;
-        }
+        return sampled;
     }
 
     @Override
     public void setSampled(boolean sampled) {
         synchronized (lock) {
             checkAlreadyEmitted();
-            this.sampled = sampled;
         }
+        this.sampled = sampled;
     }
 
     @Override

diff --git a/...recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java b/...recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java
@@ -67,7 +67,7 @@ public int getMaxSegmentSize() {
      */
     @Override
     public boolean requiresStreaming(Segment segment) {
-        if (segment.isSampled() && null != segment.getTotalSize()) {
+        if (segment.isSampled()) {
             return segment.getTotalSize().intValue() > maxSegmentSize;
         }
         return false;