[GitHub] [skywalking-java] mrproliu commented on a diff in pull request #479: Support keep trace profiling when cross-thread

["mrproliu (via GitHub)" <gi...@apache.org>]

mrproliu commented on code in PR #479:
URL: https://github.com/apache/skywalking-java/pull/479#discussion_r1135090183


##########
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java:
##########
@@ -72,39 +72,57 @@ public void stopProfiling() {
      *
      * @return is add profile success
      */
-    public ProfileStatusReference attemptProfiling(TracingContext tracingContext,
-                                                   String traceSegmentId,
-                                                   String firstSpanOPName) {
+    public ProfileStatusContext attemptProfiling(TracingContext tracingContext,
+                                                 String traceSegmentId,
+                                                 String firstSpanOPName) {
         // check has available slot
         final int usingSlotCount = currentProfilingCount.get();
         if (usingSlotCount >= Config.Profile.MAX_PARALLEL) {
-            return ProfileStatusReference.createWithNone();
+            return ProfileStatusContext.createWithNone();
         }
 
         // check first operation name matches
         if (!Objects.equals(task.getFirstSpanOPName(), firstSpanOPName)) {
-            return ProfileStatusReference.createWithNone();
+            return ProfileStatusContext.createWithNone();
         }
 
         // if out limit started profiling count then stop add profiling
         if (totalStartedProfilingCount.get() > task.getMaxSamplingCount()) {
-            return ProfileStatusReference.createWithNone();
+            return ProfileStatusContext.createWithNone();
         }
 
         // try to occupy slot
         if (!currentProfilingCount.compareAndSet(usingSlotCount, usingSlotCount + 1)) {
-            return ProfileStatusReference.createWithNone();
+            return ProfileStatusContext.createWithNone();
         }
 
+        ThreadProfiler profiler;
+        if ((profiler = addProfilingThread(tracingContext, traceSegmentId)) != null) {
+            return profiler.profilingStatus();
+        }
+        return ProfileStatusContext.createWithNone();
+    }
+
+    public boolean continueProfiling(TracingContext tracingContext, String traceSegmentId) {
+        // must smaller than max parallels monitoring count
+        if (currentProfilingCount.addAndGet(1) >= Config.Profile.MAX_PARALLEL) {
+            currentProfilingCount.addAndGet(-1);
+            return false;
+        }
+
+        return addProfilingThread(tracingContext, traceSegmentId) != null;
+    }

Review Comment:
   > What is the threshold for UI?
   
   For now, the UI only limits the count of the first segment. The sampling count is not verified when continue profiling the cross-thread segment. 
   I think the user only should only care about the profiling limit count of the first segment. The cross-thread segment should be kept profiling. 
   
   > what is for agent by default?
   
   The agent doesn't provide the sampling count, only receives the sampling count from OAP. At present, the agent only provides `profile.max_parallel` to limit the number of agent parallel analyses, so as to prevent dump data from affecting the execution of the business system.
   
   > Are they reasonably matched?
   
   The only thing that may be worried is that the value of `profile.max_parallel` is too small, which may cause the limited cross-thread not to be collected. However, this trade-off point, if the `profile.max_parallel` data is too large, may affect the execution of the business during profiling. If it is too small, it may lead to a lack of collected data.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org