From d5ccae28236b274fccb7f44b027b8c8e819b0f99 Mon Sep 17 00:00:00 2001
From: Anuraag Agrawal <aanuraag@amazon.co.jp>
Date: Fri, 8 Oct 2021 07:10:07 +0900
Subject: [PATCH] Don't lock segment from subsegment (#306)

* Don't lock segment from subsegment

* synchronized list
---
 .../amazonaws/xray/entities/EntityImpl.java   | 25 +++++++------------
 .../amazonaws/xray/entities/SegmentImpl.java  |  9 +++----
 .../strategy/DefaultStreamingStrategy.java    |  2 +-
 3 files changed, 13 insertions(+), 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
index 0725b648..27f6b10d 100644
--- 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
index fc4b05aa..7014fdb7 100644
--- 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/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java
index 89487815..bc3d57fe 100644
--- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java
+++ b/aws-xray-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;