Better memory consumption of the JenkinsHttpSessionLineBytesToLineNum…

…berConverter
jenkinsci · Mar 13, 2022 · 34e8ac5 · 34e8ac5
1 parent b69efd6
commit 34e8ac5
Show file tree

Hide file tree

Showing 4 changed files with 120 additions and 37 deletions.
diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/job/RunFlowNodeIdentifier.java b/src/main/java/io/jenkins/plugins/opentelemetry/job/RunFlowNodeIdentifier.java
@@ -6,14 +6,15 @@
 package io.jenkins.plugins.opentelemetry.job;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 import java.util.Objects;
 
 @Immutable
 public class RunFlowNodeIdentifier extends RunIdentifier {
     final String flowNodeId;
 
-    public RunFlowNodeIdentifier(@Nonnull String jobFullName, @Nonnull int runNumber, @Nonnull String flowNodeId) {
+    public RunFlowNodeIdentifier(@Nonnull String jobFullName, int runNumber, @Nullable String flowNodeId) {
         super(jobFullName, runNumber);
         this.flowNodeId = flowNodeId;
     }
@@ -24,7 +25,7 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) return false;
         if (!super.equals(o)) return false;
         RunFlowNodeIdentifier that = (RunFlowNodeIdentifier) o;
-        return flowNodeId.equals(that.flowNodeId);
+        return Objects.equals(flowNodeId, that.flowNodeId);
     }
 
     @Override

diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/job/log/util/LineIterator.java b/src/main/java/io/jenkins/plugins/opentelemetry/job/log/util/LineIterator.java
@@ -8,12 +8,12 @@
 import io.jenkins.plugins.opentelemetry.job.RunFlowNodeIdentifier;
 import org.kohsuke.stapler.Stapler;
 
-import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.servlet.http.HttpSession;
+import java.util.HashMap;
 import java.util.Iterator;
-import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
@@ -28,24 +28,14 @@ interface LineBytesToLineNumberConverter {
         Long getLogLineFromLogBytes(long bytes);
 
         void putLogBytesToLogLine(long bytes, long line);
-    }
-
-    class SimpleLineBytesToLineNumberConverter implements LineBytesToLineNumberConverter {
-        final ConcurrentMap<Long, Long> context = new ConcurrentHashMap<>();
 
-        @Nullable
-        @Override
-        public Long getLogLineFromLogBytes(long bytes) {
-            return context.get(bytes);
-        }
-
-        @Override
-        public void putLogBytesToLogLine(long bytes, long line) {
-            context.put(bytes, line);
-        }
     }
 
+    /**
+     * Converter gets garbage collected when the HTTP session expires
+     */
     class JenkinsHttpSessionLineBytesToLineNumberConverter implements LineBytesToLineNumberConverter {
+        public static final String HTTP_SESSION_KEY = "JenkinsHttpSessionLineBytesToLineNumberConverter";
         final String jobFullName;
         final int runNumber;
         final String flowNodeId;
@@ -59,30 +49,30 @@ public JenkinsHttpSessionLineBytesToLineNumberConverter(String jobFullName, int
         @Nullable
         @Override
         public Long getLogLineFromLogBytes(long bytes) {
-            HttpSession session = Stapler.getCurrentRequest().getSession();
-            String sessionAttributeKey = getSessionAttributeKey();
-            Map<Long, Long> context = (Map<Long, Long>) session.getAttribute(sessionAttributeKey);
-            if (context == null) {
-                return null;
-            }
-            return context.get(bytes);
+            RunFlowNodeIdentifier contextKey = new RunFlowNodeIdentifier(jobFullName, runNumber, flowNodeId);
+            return Optional
+                .ofNullable(getContext().get(contextKey))
+                .map(d -> d.get(bytes))
+                .orElse(null);
+
         }
 
         @Override
         public void putLogBytesToLogLine(long bytes, long line) {
-            HttpSession session = Stapler.getCurrentRequest().getSession();
-            String sessionAttributeKey = getSessionAttributeKey();
-            Map<Long, Long> context = (Map<Long, Long>) session.getAttribute(sessionAttributeKey);
-            if (context == null) {
-                context = new LinkedHashMap<>();
-                session.setAttribute(sessionAttributeKey, context);
-            }
-            context.put(bytes, line);
+            RunFlowNodeIdentifier contextKey = new RunFlowNodeIdentifier(jobFullName, runNumber, flowNodeId);
+            getContext().computeIfAbsent(contextKey, runFlowNodeIdentifier -> new HashMap<>()).put(bytes, line);
         }
 
-        @Nonnull
-        private String getSessionAttributeKey() {
-            return new RunFlowNodeIdentifier(jobFullName, runNumber, flowNodeId).getId();
+        Map<RunFlowNodeIdentifier, Map<Long, Long>> getContext() {
+            HttpSession session = Stapler.getCurrentRequest().getSession();
+            synchronized (session) {
+                Map<RunFlowNodeIdentifier, Map<Long, Long>> context = (Map<RunFlowNodeIdentifier, Map<Long, Long>>) session.getAttribute(HTTP_SESSION_KEY);
+                if (context == null) {
+                    context = new HashMap<>();
+                    session.setAttribute(HTTP_SESSION_KEY, context);
+                }
+                return context;
+            }
         }
     }
 }
diff --git a/...o/jenkins/plugins/opentelemetry/backend/elastic/ElasticsearchBuildLogsLineIteratorIT.java b/...o/jenkins/plugins/opentelemetry/backend/elastic/ElasticsearchBuildLogsLineIteratorIT.java
@@ -22,9 +22,14 @@
 import org.junit.Test;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Properties;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 public class ElasticsearchBuildLogsLineIteratorIT {
 
@@ -43,7 +48,20 @@ public void testElasticsearchLogsSearchIterator() throws IOException {
     @Test
     public void testStreamingInputStream() throws IOException {
         ElasticsearchBuildLogsLineIterator elasticsearchBuildLogsLineIterator = getElasticsearchLogsSearchIterator();
-        LineIterator.LineBytesToLineNumberConverter converter = new LineIterator.SimpleLineBytesToLineNumberConverter();
+        LineIterator.LineBytesToLineNumberConverter converter = new LineIterator.LineBytesToLineNumberConverter() {
+            Map<Long, Long> context = new HashMap<>();
+            @Nullable
+            @Override
+            public Long getLogLineFromLogBytes(long bytes) {
+                return context.get(bytes);
+            }
+
+            @Override
+            public void putLogBytesToLogLine(long bytes, long line) {
+                context.put(bytes, line);
+
+            }
+        };
         LineIteratorInputStream lineIteratorInputStream = new LineIteratorInputStream(elasticsearchBuildLogsLineIterator, converter, TracerProvider.noop().get("noop"));
         ByteStreams.copy(lineIteratorInputStream, System.out);
 

diff --git a/src/test/java/io/jenkins/plugins/opentelemetry/job/log/util/LineIteratorTest.java b/src/test/java/io/jenkins/plugins/opentelemetry/job/log/util/LineIteratorTest.java
@@ -0,0 +1,74 @@
+/*
+ * Copyright The Original Author or Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.jenkins.plugins.opentelemetry.job.log.util;
+
+import io.jenkins.plugins.opentelemetry.job.RunFlowNodeIdentifier;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+public class LineIteratorTest {
+
+    @Test
+    public void testJenkinsHttpSessionLineBytesToLineNumberConverter() {
+        final Map<RunFlowNodeIdentifier, Map<Long, Long>> context = new HashMap<>();
+
+
+        LineIterator.JenkinsHttpSessionLineBytesToLineNumberConverter converterJob1Run1 = new LineIterator.JenkinsHttpSessionLineBytesToLineNumberConverter("job-1", 1, null) {
+            @Override
+            Map<RunFlowNodeIdentifier, Map<Long, Long>> getContext() {
+                return context;
+            }
+        };
+
+        converterJob1Run1.putLogBytesToLogLine(100, 111);
+        converterJob1Run1.putLogBytesToLogLine(1_000, 1_111);
+        converterJob1Run1.putLogBytesToLogLine(10_000, 11_111);
+
+        LineIterator.JenkinsHttpSessionLineBytesToLineNumberConverter converterJob1Run1FlowNode1 = new LineIterator.JenkinsHttpSessionLineBytesToLineNumberConverter("job-1", 1, "1") {
+            @Override
+            Map<RunFlowNodeIdentifier, Map<Long, Long>> getContext() {
+                return context;
+            }
+        };
+        converterJob1Run1FlowNode1.putLogBytesToLogLine(100, 7_111);
+        converterJob1Run1FlowNode1.putLogBytesToLogLine(1_000, 71_111);
+        converterJob1Run1FlowNode1.putLogBytesToLogLine(10_000, 711_111);
+
+        LineIterator.JenkinsHttpSessionLineBytesToLineNumberConverter converterJob2Run2 = new LineIterator.JenkinsHttpSessionLineBytesToLineNumberConverter("job-2", 2, null) {
+            @Override
+            Map<RunFlowNodeIdentifier, Map<Long, Long>> getContext() {
+                return context;
+            }
+        };
+
+        converterJob2Run2.putLogBytesToLogLine(100, 222);
+        converterJob2Run2.putLogBytesToLogLine(1_000, 2_222);
+        converterJob2Run2.putLogBytesToLogLine(10_000, 22_222);
+
+        LineIterator.JenkinsHttpSessionLineBytesToLineNumberConverter converterJob2Run2FlowNode2 = new LineIterator.JenkinsHttpSessionLineBytesToLineNumberConverter("job-2", 2, "2") {
+            @Override
+            Map<RunFlowNodeIdentifier, Map<Long, Long>> getContext() {
+                return context;
+            }
+        };
+        converterJob2Run2FlowNode2.putLogBytesToLogLine(100, 7_222);
+        converterJob2Run2FlowNode2.putLogBytesToLogLine(1_000, 72_222);
+        converterJob2Run2FlowNode2.putLogBytesToLogLine(10_000, 722_222);
+
+
+        assertEquals(Long.valueOf(111), converterJob1Run1.getLogLineFromLogBytes(100));
+        assertEquals(Long.valueOf(7_111), converterJob1Run1FlowNode1.getLogLineFromLogBytes(100));
+        assertEquals(Long.valueOf(222), converterJob2Run2.getLogLineFromLogBytes(100));
+        assertEquals(Long.valueOf(7_222), converterJob2Run2FlowNode2.getLogLineFromLogBytes(100));
+        assertEquals(Long.valueOf(722_222), converterJob2Run2FlowNode2.getLogLineFromLogBytes(10_000));
+
+    }
+
+}