[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-18 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r368215435
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ThreadProfiler.java
 ##
 @@ -0,0 +1,153 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.agent.core.profile;
+
+import com.google.common.base.Objects;
+import org.apache.skywalking.apm.agent.core.conf.Config;
+import org.apache.skywalking.apm.agent.core.context.TracingContext;
+import org.apache.skywalking.apm.agent.core.context.ids.ID;
+
+import java.util.ArrayList;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * @author MrPro
+ */
+public class ThreadProfiler {
+
+// current tracing context
+private final TracingContext tracingContext;
+// current tracing segment id
+private final ID traceSegmentId;
+// need to profiling thread
+private final Thread profilingThread;
+// profiling execution context
+private final ProfileTaskExecutionContext executionContext;
+
+// profiling time
+private long profilingStartTime;
+private long profilingMaxTimeMills;
+
+// after min duration threshold check, it will start dump
+private ProfilingStatus profilingStatus = ProfilingStatus.READY;
+// thread dump sequence
+private int dumpSequence = 0;
+
+public ThreadProfiler(TracingContext tracingContext, ID traceSegmentId, 
Thread profilingThread, ProfileTaskExecutionContext executionContext) {
+this.tracingContext = tracingContext;
+this.traceSegmentId = traceSegmentId;
+this.profilingThread = profilingThread;
+this.executionContext = executionContext;
+this.profilingMaxTimeMills = 
TimeUnit.MINUTES.toMillis(Config.Profile.MAX_DURATION);
+}
+
+/**
+ * If tracing start time greater than {@link 
ProfileTask#getMinDurationThreshold()}, then start to profiling trace
+ */
+public void startProfilingIfNeed() {
+if (System.currentTimeMillis() - tracingContext.createTime() > 
executionContext.getTask().getMinDurationThreshold()) {
+this.profilingStartTime = System.currentTimeMillis();
+this.profilingStatus = ProfilingStatus.PROFILING;
+}
+}
+
+/**
+ * Stop profiling status
+ */
+public void stopProfiling() {
+this.profilingStatus = ProfilingStatus.STOPPED;
+}
+
+/**
+ * dump tracing thread and build thread snapshot
+ *
+ * @return snapshot, if null means dump snapshot error, should stop it
+ */
+public TracingThreadSnapshot buildSnapshot() {
+if (!isProfilingProfilingContinuable()) {
+return null;
+}
+
+long currentTime = System.currentTimeMillis();
+// dump thread
+StackTraceElement[] stackTrace;
+try {
+stackTrace = profilingThread.getStackTrace();
+
+// stack depth is zero, means thread is already run finished
+if (stackTrace.length == 0) {
+return null;
+}
+} catch (Exception e) {
+// dump error ignore and make this profiler stop
+return null;
+}
+
+// if is first dump, check is can start profiling
+if (dumpSequence == 0 && (!executionContext.isStartProfileable())) {
+return null;
+}
+
+int dumpElementCount = Math.min(stackTrace.length, 
Config.Profile.DUMP_MAX_STACK_DEPTH);
+
+// use inverted order, because thread dump is start with bottom
+final ArrayList stackList = new ArrayList<>(dumpElementCount);
+for (int i = dumpElementCount - 1; i >= 0; i--) {
+stackList.add(buildStackElementCodeSignature(stackTrace[i]));
+}
+
+String taskId = executionContext.getTask().getTaskId();
+return new TracingThreadSnapshot(taskId, traceSegmentId, 
dumpSequence++, currentTime, stackList);
+}
+
+/**
+ * build thread stack element code signature
+ *
+ * @return code sign: className.methodName:lineNumber
+ */
+private String 

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-17 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r368192761
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -30,22 +37,139 @@
 // task data
 private final ProfileTask task;
 
-// task real start time
-private final long startTime;
+// record current profiling count, use this to check has available profile 
slot
+private final AtomicInteger currentProfilingCount = new AtomicInteger(0);
+
+// profiling segment slot
+private volatile ThreadProfiler[] profilingSegmentSlots = new 
ThreadProfiler[Config.Profile.MAX_PARALLEL];
+
+// current profiling execution future
+private volatile Future profilingFuture;
+
+// total started profiling tracing context count
+private final AtomicInteger totalStartedProfilingCount = new 
AtomicInteger(0);
 
-public ProfileTaskExecutionContext(ProfileTask task, long startTime) {
+public ProfileTaskExecutionContext(ProfileTask task) {
 this.task = task;
-this.startTime = startTime;
+}
+
+/**
+ * start profiling this task
+ * @param executorService
+ */
+public void startProfiling(ExecutorService executorService) {
+profilingFuture = executorService.submit(new ProfileThread(this));
+}
+
+/**
+ * stop profiling
+ */
+public void stopProfiling() {
+if (profilingFuture != null) {
+profilingFuture.cancel(true);
+}
+}
+
+/**
+ * check have available slot to profile and add it
+ * @param tracingContext
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean attemptProfiling(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+// check has available slot
+final int usingSlotCount = currentProfilingCount.get();
+if (usingSlotCount >= Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+return false;
+}
+
+return tryToAttemptProfiling(tracingContext, traceSegmentId, 
firstSpanOPName, usingSlotCount);
+}
+
+
+/**
+ * profiling recheck
+ * @param tracingContext
+ * @param traceSegmentId
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean profilingRecheck(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+boolean alreadyProfiling = tracingContext.isProfiling();
+
+// not profiling and not available slot don't check anymore
+int usingSlotCount = currentProfilingCount.get();
+if (!alreadyProfiling && usingSlotCount >= 
Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+if (alreadyProfiling) {
+if (stopTracingProfile(tracingContext)) {
+// reduce total started profiling count when status is 
profiling
+totalStartedProfilingCount.addAndGet(-1);
+}
+}
+return false;
+} else if (alreadyProfiling) {
+return true;
+}
+
+// not profiling, try to occupy slot
+return tryToAttemptProfiling(tracingContext, traceSegmentId, 
firstSpanOPName, usingSlotCount);
+}
+
+/**
+ * find tracing context and clear on slot
+ *
+ * @param tracingContext
+ *
+ * @return current profiler is already start profiling
+ */
+public boolean stopTracingProfile(TracingContext tracingContext) {
+// find current tracingContext and clear it
+boolean isProfilingStarted = false;
+for (int slot = 0; slot < profilingSegmentSlots.length; slot++) {
+ThreadProfiler currentProfiler = profilingSegmentSlots[slot];
+if (currentProfiler != null && 
currentProfiler.matches(tracingContext)) {
+profilingSegmentSlots[slot] = null;
+
+// setting stop running
+isProfilingStarted = currentProfiler.stopProfiling();
+currentProfilingCount.addAndGet(-1);
+
+profilingSegmentSlots = profilingSegmentSlots;
 
 Review comment:
   Make sense for switching to `AtomicReferenceArray` in this case.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-17 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r368015051
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -30,22 +37,139 @@
 // task data
 private final ProfileTask task;
 
-// task real start time
-private final long startTime;
+// record current profiling count, use this to check has available profile 
slot
+private final AtomicInteger currentProfilingCount = new AtomicInteger(0);
+
+// profiling segment slot
+private volatile ThreadProfiler[] profilingSegmentSlots = new 
ThreadProfiler[Config.Profile.MAX_PARALLEL];
+
+// current profiling execution future
+private volatile Future profilingFuture;
+
+// total started profiling tracing context count
+private final AtomicInteger totalStartedProfilingCount = new 
AtomicInteger(0);
 
-public ProfileTaskExecutionContext(ProfileTask task, long startTime) {
+public ProfileTaskExecutionContext(ProfileTask task) {
 this.task = task;
-this.startTime = startTime;
+}
+
+/**
+ * start profiling this task
+ * @param executorService
+ */
+public void startProfiling(ExecutorService executorService) {
+profilingFuture = executorService.submit(new ProfileThread(this));
+}
+
+/**
+ * stop profiling
+ */
+public void stopProfiling() {
+if (profilingFuture != null) {
+profilingFuture.cancel(true);
+}
+}
+
+/**
+ * check have available slot to profile and add it
+ * @param tracingContext
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean attemptProfiling(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+// check has available slot
+final int usingSlotCount = currentProfilingCount.get();
+if (usingSlotCount >= Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+return false;
+}
+
+return tryToAttemptProfiling(tracingContext, traceSegmentId, 
firstSpanOPName, usingSlotCount);
+}
+
+
+/**
+ * profiling recheck
+ * @param tracingContext
+ * @param traceSegmentId
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean profilingRecheck(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+boolean alreadyProfiling = tracingContext.isProfiling();
+
+// not profiling and not available slot don't check anymore
+int usingSlotCount = currentProfilingCount.get();
+if (!alreadyProfiling && usingSlotCount >= 
Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+if (alreadyProfiling) {
+if (stopTracingProfile(tracingContext)) {
+// reduce total started profiling count when status is 
profiling
+totalStartedProfilingCount.addAndGet(-1);
+}
+}
+return false;
+} else if (alreadyProfiling) {
+return true;
+}
+
+// not profiling, try to occupy slot
+return tryToAttemptProfiling(tracingContext, traceSegmentId, 
firstSpanOPName, usingSlotCount);
+}
+
+/**
+ * find tracing context and clear on slot
+ *
+ * @param tracingContext
+ *
+ * @return current profiler is already start profiling
+ */
+public boolean stopTracingProfile(TracingContext tracingContext) {
+// find current tracingContext and clear it
+boolean isProfilingStarted = false;
+for (int slot = 0; slot < profilingSegmentSlots.length; slot++) {
+ThreadProfiler currentProfiler = profilingSegmentSlots[slot];
+if (currentProfiler != null && 
currentProfiler.matches(tracingContext)) {
+profilingSegmentSlots[slot] = null;
+
+// setting stop running
+isProfilingStarted = currentProfiler.stopProfiling();
+currentProfilingCount.addAndGet(-1);
+
+profilingSegmentSlots = profilingSegmentSlots;
 
 Review comment:
   I remember that did this in different way. Can't remember the 
detail.Personally, I just use the easy way. 
@mrproliu you could add doc, but don't use the link to that blog.


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.
 
For queries about this service, please 

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-17 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367980760
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -30,22 +37,139 @@
 // task data
 private final ProfileTask task;
 
-// task real start time
-private final long startTime;
+// record current profiling count, use this to check has available profile 
slot
+private final AtomicInteger currentProfilingCount = new AtomicInteger(0);
+
+// profiling segment slot
+private volatile ThreadProfiler[] profilingSegmentSlots = new 
ThreadProfiler[Config.Profile.MAX_PARALLEL];
+
+// current profiling execution future
+private volatile Future profilingFuture;
+
+// total started profiling tracing context count
+private final AtomicInteger totalStartedProfilingCount = new 
AtomicInteger(0);
 
-public ProfileTaskExecutionContext(ProfileTask task, long startTime) {
+public ProfileTaskExecutionContext(ProfileTask task) {
 this.task = task;
-this.startTime = startTime;
+}
+
+/**
+ * start profiling this task
+ * @param executorService
+ */
+public void startProfiling(ExecutorService executorService) {
+profilingFuture = executorService.submit(new ProfileThread(this));
+}
+
+/**
+ * stop profiling
+ */
+public void stopProfiling() {
+if (profilingFuture != null) {
+profilingFuture.cancel(true);
+}
+}
+
+/**
+ * check have available slot to profile and add it
+ * @param tracingContext
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean attemptProfiling(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+// check has available slot
+final int usingSlotCount = currentProfilingCount.get();
+if (usingSlotCount >= Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+return false;
+}
+
+return tryToAttemptProfiling(tracingContext, traceSegmentId, 
firstSpanOPName, usingSlotCount);
+}
+
+
+/**
+ * profiling recheck
+ * @param tracingContext
+ * @param traceSegmentId
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean profilingRecheck(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+boolean alreadyProfiling = tracingContext.isProfiling();
+
+// not profiling and not available slot don't check anymore
+int usingSlotCount = currentProfilingCount.get();
+if (!alreadyProfiling && usingSlotCount >= 
Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+if (alreadyProfiling) {
+if (stopTracingProfile(tracingContext)) {
+// reduce total started profiling count when status is 
profiling
+totalStartedProfilingCount.addAndGet(-1);
+}
+}
+return false;
+} else if (alreadyProfiling) {
+return true;
+}
+
+// not profiling, try to occupy slot
+return tryToAttemptProfiling(tracingContext, traceSegmentId, 
firstSpanOPName, usingSlotCount);
+}
+
+/**
+ * find tracing context and clear on slot
+ *
+ * @param tracingContext
+ *
+ * @return current profiler is already start profiling
+ */
+public boolean stopTracingProfile(TracingContext tracingContext) {
+// find current tracingContext and clear it
+boolean isProfilingStarted = false;
+for (int slot = 0; slot < profilingSegmentSlots.length; slot++) {
+ThreadProfiler currentProfiler = profilingSegmentSlots[slot];
+if (currentProfiler != null && 
currentProfiler.matches(tracingContext)) {
+profilingSegmentSlots[slot] = null;
+
+// setting stop running
+isProfilingStarted = currentProfiler.stopProfiling();
+currentProfilingCount.addAndGet(-1);
+
+profilingSegmentSlots = profilingSegmentSlots;
 
 Review comment:
   You are changing the element of an array, the volatile would work unless we 
do this.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-17 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367980372
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionService.java
 ##
 @@ -151,7 +202,7 @@ public long getLastCommandCreateTime() {
  */
 private String checkProfileTaskSuccess(ProfileTask task) {
 
 Review comment:
   In this case, I prefer to return a `Result` class, rather than exception. 
Profiling is happening in a high payload scenario, but Exception is a heavy OP. 
We should avoid it.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-17 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367979687
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -30,22 +37,139 @@
 // task data
 private final ProfileTask task;
 
-// task real start time
-private final long startTime;
+// record current profiling count, use this to check has available profile 
slot
+private final AtomicInteger currentProfilingCount = new AtomicInteger(0);
+
+// profiling segment slot
+private volatile ThreadProfiler[] profilingSegmentSlots = new 
ThreadProfiler[Config.Profile.MAX_PARALLEL];
+
+// current profiling execution future
+private volatile Future profilingFuture;
+
+// total started profiling tracing context count
+private final AtomicInteger totalStartedProfilingCount = new 
AtomicInteger(0);
 
-public ProfileTaskExecutionContext(ProfileTask task, long startTime) {
+public ProfileTaskExecutionContext(ProfileTask task) {
 this.task = task;
-this.startTime = startTime;
+}
+
+/**
+ * start profiling this task
+ * @param executorService
+ */
+public void startProfiling(ExecutorService executorService) {
+profilingFuture = executorService.submit(new ProfileThread(this));
+}
+
+/**
+ * stop profiling
+ */
+public void stopProfiling() {
+if (profilingFuture != null) {
+profilingFuture.cancel(true);
+}
+}
+
+/**
+ * check have available slot to profile and add it
+ * @param tracingContext
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean attemptProfiling(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+// check has available slot
+final int usingSlotCount = currentProfilingCount.get();
+if (usingSlotCount >= Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+return false;
+}
+
+return tryToAttemptProfiling(tracingContext, traceSegmentId, 
firstSpanOPName, usingSlotCount);
+}
+
+
+/**
+ * profiling recheck
+ * @param tracingContext
+ * @param traceSegmentId
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean profilingRecheck(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+boolean alreadyProfiling = tracingContext.isProfiling();
+
+// not profiling and not available slot don't check anymore
+int usingSlotCount = currentProfilingCount.get();
+if (!alreadyProfiling && usingSlotCount >= 
Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+if (alreadyProfiling) {
+if (stopTracingProfile(tracingContext)) {
+// reduce total started profiling count when status is 
profiling
+totalStartedProfilingCount.addAndGet(-1);
+}
+}
+return false;
+} else if (alreadyProfiling) {
+return true;
+}
+
+// not profiling, try to occupy slot
+return tryToAttemptProfiling(tracingContext, traceSegmentId, 
firstSpanOPName, usingSlotCount);
+}
+
+/**
+ * find tracing context and clear on slot
+ *
+ * @param tracingContext
+ *
+ * @return current profiler is already start profiling
+ */
+public boolean stopTracingProfile(TracingContext tracingContext) {
+// find current tracingContext and clear it
+boolean isProfilingStarted = false;
+for (int slot = 0; slot < profilingSegmentSlots.length; slot++) {
+ThreadProfiler currentProfiler = profilingSegmentSlots[slot];
+if (currentProfiler != null && 
currentProfiler.matches(tracingContext)) {
+profilingSegmentSlots[slot] = null;
+
+// setting stop running
+isProfilingStarted = currentProfiler.stopProfiling();
+currentProfilingCount.addAndGet(-1);
+
+profilingSegmentSlots = profilingSegmentSlots;
 
 Review comment:
   You need to follow prefer review comments :) This is for activating 
`volatile` :P 


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-17 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367929902
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -30,22 +37,139 @@
 // task data
 private final ProfileTask task;
 
-// task real start time
-private final long startTime;
+// record current profiling count, use this to check has available profile 
slot
+private final AtomicInteger currentProfilingCount = new AtomicInteger(0);
+
+// profiling segment slot
+private volatile ThreadProfiler[] profilingSegmentSlots = new 
ThreadProfiler[Config.Profile.MAX_PARALLEL];
+
+// current profiling execution future
+private volatile Future profilingFuture;
+
+// total started profiling tracing context count
+private final AtomicInteger totalStartedProfilingCount = new 
AtomicInteger(0);
 
-public ProfileTaskExecutionContext(ProfileTask task, long startTime) {
+public ProfileTaskExecutionContext(ProfileTask task) {
 this.task = task;
-this.startTime = startTime;
+}
+
+/**
+ * start profiling this task
+ * @param executorService
+ */
+public void startProfiling(ExecutorService executorService) {
+profilingFuture = executorService.submit(new ProfileThread(this));
+}
+
+/**
+ * stop profiling
+ */
+public void stopProfiling() {
+if (profilingFuture != null) {
+profilingFuture.cancel(true);
+}
+}
+
+/**
+ * check have available slot to profile and add it
+ * @param tracingContext
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean attemptProfiling(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+// check has available slot
+final int usingSlotCount = currentProfilingCount.get();
+if (usingSlotCount >= Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+return false;
+}
+
+return tryToAttemptProfiling(tracingContext, traceSegmentId, 
firstSpanOPName, usingSlotCount);
+}
+
+
+/**
+ * profiling recheck
+ * @param tracingContext
+ * @param traceSegmentId
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean profilingRecheck(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+boolean alreadyProfiling = tracingContext.isProfiling();
+
+// not profiling and not available slot don't check anymore
+int usingSlotCount = currentProfilingCount.get();
+if (!alreadyProfiling && usingSlotCount >= 
Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+if (alreadyProfiling) {
+if (stopTracingProfile(tracingContext)) {
+// reduce total started profiling count when status is 
profiling
+totalStartedProfilingCount.addAndGet(-1);
+}
+}
+return false;
+} else if (alreadyProfiling) {
+return true;
+}
+
+// not profiling, try to occupy slot
+return tryToAttemptProfiling(tracingContext, traceSegmentId, 
firstSpanOPName, usingSlotCount);
+}
+
+/**
+ * find tracing context and clear on slot
+ *
+ * @param tracingContext
+ *
+ * @return current profiler is already start profiling
+ */
+public boolean stopTracingProfile(TracingContext tracingContext) {
+// find current tracingContext and clear it
+boolean isProfilingStarted = false;
+for (int slot = 0; slot < profilingSegmentSlots.length; slot++) {
+ThreadProfiler currentProfiler = profilingSegmentSlots[slot];
+if (currentProfiler != null && 
currentProfiler.matches(tracingContext)) {
+profilingSegmentSlots[slot] = null;
+
+// setting stop running
+isProfilingStarted = currentProfiler.stopProfiling();
+currentProfilingCount.addAndGet(-1);
+
+profilingSegmentSlots = profilingSegmentSlots;
+break;
+}
+}
+
+return isProfilingStarted;
 }
 
 public ProfileTask getTask() {
 return task;
 }
 
-public long getStartTime() {
-return startTime;
+public ThreadProfiler[] threadProfilerSlots() {
+return profilingSegmentSlots;
 }
 
+public boolean isStartProfileable(ThreadProfiler profiler) {
+// check is out of max sampling count check
+if 

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-17 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367933510
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -58,4 +182,27 @@ public boolean equals(Object o) {
 public int hashCode() {
 return Objects.hash(task);
 }
+
+private boolean tryToAttemptProfiling(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName, int currentUsingSlotCount) {
+// if out limit started profiling count then stop add profiling
+if (totalStartedProfilingCount.get() > task.getMaxSamplingCount()) {
+return false;
+}
+
+// try to occupy slot
+if (!currentProfilingCount.compareAndSet(currentUsingSlotCount, 
currentUsingSlotCount + 1)) {
+return false;
+}
+
+final ThreadProfiler segmentContext = new 
ThreadProfiler(tracingContext, traceSegmentId, Thread.currentThread(), this);
 
 Review comment:
   This isn't segmentContext, please rename.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-17 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367930556
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -30,22 +37,139 @@
 // task data
 private final ProfileTask task;
 
-// task real start time
-private final long startTime;
+// record current profiling count, use this to check has available profile 
slot
+private final AtomicInteger currentProfilingCount = new AtomicInteger(0);
+
+// profiling segment slot
+private volatile ThreadProfiler[] profilingSegmentSlots = new 
ThreadProfiler[Config.Profile.MAX_PARALLEL];
+
+// current profiling execution future
+private volatile Future profilingFuture;
+
+// total started profiling tracing context count
+private final AtomicInteger totalStartedProfilingCount = new 
AtomicInteger(0);
 
-public ProfileTaskExecutionContext(ProfileTask task, long startTime) {
+public ProfileTaskExecutionContext(ProfileTask task) {
 this.task = task;
-this.startTime = startTime;
+}
+
+/**
+ * start profiling this task
+ * @param executorService
+ */
+public void startProfiling(ExecutorService executorService) {
+profilingFuture = executorService.submit(new ProfileThread(this));
+}
+
+/**
+ * stop profiling
+ */
+public void stopProfiling() {
+if (profilingFuture != null) {
+profilingFuture.cancel(true);
+}
+}
+
+/**
+ * check have available slot to profile and add it
+ * @param tracingContext
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean attemptProfiling(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+// check has available slot
+final int usingSlotCount = currentProfilingCount.get();
+if (usingSlotCount >= Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+return false;
+}
+
+return tryToAttemptProfiling(tracingContext, traceSegmentId, 
firstSpanOPName, usingSlotCount);
+}
+
+
+/**
+ * profiling recheck
+ * @param tracingContext
+ * @param traceSegmentId
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean profilingRecheck(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+boolean alreadyProfiling = tracingContext.isProfiling();
+
+// not profiling and not available slot don't check anymore
+int usingSlotCount = currentProfilingCount.get();
+if (!alreadyProfiling && usingSlotCount >= 
Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+if (alreadyProfiling) {
 
 Review comment:
   Otherwise, if I set a profiling task targeting a Tomcat endpoint name, it 
will never execute as SpringMVC must override it


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-17 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367924796
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -58,4 +182,27 @@ public boolean equals(Object o) {
 public int hashCode() {
 return Objects.hash(task);
 }
+
+private boolean tryToAttemptProfiling(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName, int currentUsingSlotCount) {
 
 Review comment:
   Merge `tryToAttemptProfiling` -> `attemptProfiling`. Similar name, always 
use in the same time.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-17 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367918152
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -30,22 +37,139 @@
 // task data
 private final ProfileTask task;
 
-// task real start time
-private final long startTime;
+// record current profiling count, use this to check has available profile 
slot
+private final AtomicInteger currentProfilingCount = new AtomicInteger(0);
+
+// profiling segment slot
+private volatile ThreadProfiler[] profilingSegmentSlots = new 
ThreadProfiler[Config.Profile.MAX_PARALLEL];
 
 Review comment:
   Same as before, don't use Config in the field initialization. Use it in the 
constructor.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-17 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367930307
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -30,22 +37,139 @@
 // task data
 private final ProfileTask task;
 
-// task real start time
-private final long startTime;
+// record current profiling count, use this to check has available profile 
slot
+private final AtomicInteger currentProfilingCount = new AtomicInteger(0);
+
+// profiling segment slot
+private volatile ThreadProfiler[] profilingSegmentSlots = new 
ThreadProfiler[Config.Profile.MAX_PARALLEL];
+
+// current profiling execution future
+private volatile Future profilingFuture;
+
+// total started profiling tracing context count
+private final AtomicInteger totalStartedProfilingCount = new 
AtomicInteger(0);
 
-public ProfileTaskExecutionContext(ProfileTask task, long startTime) {
+public ProfileTaskExecutionContext(ProfileTask task) {
 this.task = task;
-this.startTime = startTime;
+}
+
+/**
+ * start profiling this task
+ * @param executorService
+ */
+public void startProfiling(ExecutorService executorService) {
+profilingFuture = executorService.submit(new ProfileThread(this));
+}
+
+/**
+ * stop profiling
+ */
+public void stopProfiling() {
+if (profilingFuture != null) {
+profilingFuture.cancel(true);
+}
+}
+
+/**
+ * check have available slot to profile and add it
+ * @param tracingContext
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean attemptProfiling(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+// check has available slot
+final int usingSlotCount = currentProfilingCount.get();
+if (usingSlotCount >= Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+return false;
+}
+
+return tryToAttemptProfiling(tracingContext, traceSegmentId, 
firstSpanOPName, usingSlotCount);
+}
+
+
+/**
+ * profiling recheck
+ * @param tracingContext
+ * @param traceSegmentId
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean profilingRecheck(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+boolean alreadyProfiling = tracingContext.isProfiling();
+
+// not profiling and not available slot don't check anymore
+int usingSlotCount = currentProfilingCount.get();
+if (!alreadyProfiling && usingSlotCount >= 
Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+if (alreadyProfiling) {
 
 Review comment:
   Once the profile starts, it should not stop.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-16 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367747127
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -174,6 +174,9 @@ private boolean tryToAttemptProfiling(TracingContext 
tracingContext, ID traceSeg
 for (int slot = 0; slot < profilingSegmentSlots.length; slot++) {
 if (profilingSegmentSlots[slot] == null) {
 profilingSegmentSlots[slot] = segmentContext;
+
+// see 
https://www.javamex.com/tutorials/volatile_arrays.shtml, solution 2
 
 Review comment:
   Don't put the link here. The article is just reference, we don't have the 
license to broadcast it.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-16 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367272230
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -58,4 +163,20 @@ public boolean equals(Object o) {
 public int hashCode() {
 return Objects.hash(task);
 }
+
+private boolean tryToAttemptProfiling(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName, int currentUsingSlotCount) {
+// try to occupy slot
+if (!currentProfilingCount.compareAndSet(currentUsingSlotCount, 
currentUsingSlotCount + 1)) {
+return false;
+}
+
+final ThreadProfiler segmentContext = new 
ThreadProfiler(tracingContext, traceSegmentId, Thread.currentThread(), this);
+for (int slot = 0; slot < profilingSegmentSlots.length; slot++) {
+if (profilingSegmentSlots[slot] == null) {
+profilingSegmentSlots[slot] = segmentContext;
 
 Review comment:
   I think you missed the profilingSegmentSlots resign to make `volatile` works.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-16 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367266011
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/trace/AbstractTracingSpan.java
 ##
 @@ -203,6 +203,9 @@ public AbstractTracingSpan errorOccurred() {
 public AbstractTracingSpan setOperationName(String operationName) {
 this.operationName = operationName;
 this.operationId = DictionaryUtil.nullValue();
+
+// recheck profiling status
+ContextManager.profilingRecheck(this, operationName);
 
 Review comment:
   I think we don't need `ContextManager#profilingRecheck`. There is a ref of 
`tracingContext` in the span, check `AbstractTracingSpan`. You should use it, 
it is better to keep `profilingRecheck` in private, right?


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-16 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r367265071
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java
 ##
 @@ -462,16 +485,34 @@ public boolean stopSpan(AbstractSpan span) {
 finish();
 }
 
+@Override
+public void profilingRecheck(AbstractSpan span, String operationName) {
+// only recheck first span
+if (span.getSpanId() != 0) {
+return;
+}
+
+profiling = PROFILE_TASK_EXECUTION_SERVICE.profilingRecheck(this, 
segment.getTraceSegmentId(), operationName);
+}
+
 /**
  * Finish this context, and notify all {@link TracingContextListener}s, 
managed by {@link
- * TracingContext.ListenerManager}
+ * TracingContext.ListenerManager} and {@link 
TracingContext.TracingThreadListenerManager}
  */
 private void finish() {
 if (isRunningInAsyncMode) {
 asyncFinishLock.lock();
 }
 try {
-if (activeSpanStack.isEmpty() && running && (!isRunningInAsyncMode 
|| asyncSpanCounter.get() == 0)) {
+boolean tracingFinishInMainThread = activeSpanStack.isEmpty() && 
running;
 
 Review comment:
   `tracingFinishInMainThread` -> `isFinishedInMainThread`


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-15 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r366862127
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -30,20 +37,101 @@
 // task data
 private final ProfileTask task;
 
-// task real start time
-private final long startTime;
+// record current profiling count, use this to check has available profile 
slot
+private final AtomicInteger currentProfilingCount = new AtomicInteger(0);
+
+// profiling segment slot
+private final ThreadProfiler[] profilingSegmentSlot = new 
ThreadProfiler[Config.Profile.MAX_PARALLEL];
 
 Review comment:
   `threadProfilerSlot`  -> `threadProfilerSlots`


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-15 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r366864228
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileTaskExecutionContext.java
 ##
 @@ -30,20 +37,101 @@
 // task data
 private final ProfileTask task;
 
-// task real start time
-private final long startTime;
+// record current profiling count, use this to check has available profile 
slot
+private final AtomicInteger currentProfilingCount = new AtomicInteger(0);
+
+// profiling segment slot
+private final ThreadProfiler[] profilingSegmentSlot = new 
ThreadProfiler[Config.Profile.MAX_PARALLEL];
+
+// current profiling execution future
+private volatile Future profilingFuture;
 
-public ProfileTaskExecutionContext(ProfileTask task, long startTime) {
+public ProfileTaskExecutionContext(ProfileTask task) {
 this.task = task;
-this.startTime = startTime;
+}
+
+/**
+ * start profiling this task
+ * @param executorService
+ */
+public void startProfiling(ExecutorService executorService) {
+profilingFuture = executorService.submit(new ProfileThread(this));
+}
+
+/**
+ * stop profiling
+ */
+public void stopProfiling() {
+if (profilingFuture != null) {
+profilingFuture.cancel(true);
+}
+}
+
+/**
+ * check have available slot to profile and add it
+ * @param tracingContext
+ * @param firstSpanOPName
+ * @return
+ */
+public boolean attemptProfiling(TracingContext tracingContext, ID 
traceSegmentId, String firstSpanOPName) {
+// check has available slot
+final int usingSlotCount = currentProfilingCount.get();
+if (usingSlotCount >= Config.Profile.MAX_PARALLEL) {
+return false;
+}
+
+// check first operation name matches
+if (!Objects.equals(task.getFistSpanOPName(), firstSpanOPName)) {
+return false;
+}
+
+// try to occupy slot
+if (!currentProfilingCount.compareAndSet(usingSlotCount, 
usingSlotCount + 1)) {
+return false;
+}
+
+final ThreadProfiler segmentContext = new 
ThreadProfiler(tracingContext, traceSegmentId, Thread.currentThread(), this);
+for (int slot = 0; slot < profilingSegmentSlot.length; slot++) {
+if (profilingSegmentSlot[slot] == null) {
+profilingSegmentSlot[slot] = segmentContext;
+break;
+}
+}
+return true;
+}
+
+/**
+ * find tracing context and clear on slot
+ *
+ * @param tracingContext
+ */
+public void stopTracingProfile(TracingContext tracingContext) {
+// find current tracingContext and clear it
+boolean find = false;
+for (int slot = 0; slot < profilingSegmentSlot.length; slot++) {
+ThreadProfiler currentProfiler = profilingSegmentSlot[slot];
+if (currentProfiler != null && 
currentProfiler.matches(tracingContext)) {
+profilingSegmentSlot[slot] = null;
 
 Review comment:
   As this method could be called in another thread, you need to set the whole 
array again, and add `volatile`, read solution 2, 
https://www.javamex.com/tutorials/volatile_arrays.shtml


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-15 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r366854851
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java
 ##
 @@ -497,6 +510,11 @@ private void finish() {
 TracingContext.ListenerManager.notifyFinish(finishedSegment);
 
 running = false;
+
+/**
+ * Notify current tracing context main thread has already 
execute finished.
 
 Review comment:
   This notification should not be in `(!isRunningInAsyncMode || 
asyncSpanCounter.get() == 0))`. The condition means async finished.  You just 
need `activeSpanStack.isEmpty() && running`.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-15 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r366855307
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/trace/TraceSegment.java
 ##
 @@ -74,6 +74,9 @@
 
 private final long createTime;
 
+// segment is profiling
+private volatile boolean profiling;
 
 Review comment:
   Why does the segment still need this? This should be moved into 
`TracingContext`.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-15 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r366853949
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java
 ##
 @@ -92,15 +93,27 @@
 
 private volatile boolean running;
 
+private final long createTime;
+
+/**
+ * profiling status
+ */
+private final boolean profiling;
+
 /**
  * Initialize all fields with default value.
  */
-TracingContext() {
+TracingContext(String firstOPName) {
 this.segment = new TraceSegment();
 this.spanIdGenerator = 0;
 samplingService = 
ServiceManager.INSTANCE.findService(SamplingService.class);
 isRunningInAsyncMode = false;
+createTime = System.currentTimeMillis();
 running = true;
+
+// profiling status
+final ProfileTaskExecutionService profileTaskExecutionService = 
ServiceManager.INSTANCE.findService(ProfileTaskExecutionService.class);
 
 Review comment:
   You don't need to `findService` every time. Please add a field to hold the 
ref.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-15 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r366854422
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java
 ##
 @@ -497,6 +510,11 @@ private void finish() {
 TracingContext.ListenerManager.notifyFinish(finishedSegment);
 
 running = false;
+
+/**
+ * Notify current tracing context main thread has already 
execute finished.
 
 Review comment:
   `Notify current tracing context main thread has already execute finished.` 
-> `Notify after tracing finished in the main thread`.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-15 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r366867575
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileThread.java
 ##
 @@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.agent.core.profile;
+
+import org.apache.skywalking.apm.agent.core.boot.ServiceManager;
+import org.apache.skywalking.apm.agent.core.logging.api.ILog;
+import org.apache.skywalking.apm.agent.core.logging.api.LogManager;
+
+/**
+ * Profile task process thread, dump segment executing thread stack.
 
 Review comment:
   `segment` should not exist. Dump the executing thread stack.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [skywalking] wu-sheng commented on a change in pull request #4220: sniffer processing profile task and report status and snapshot

2020-01-15 Thread GitBox
wu-sheng commented on a change in pull request #4220: sniffer processing 
profile task and report status and snapshot
URL: https://github.com/apache/skywalking/pull/4220#discussion_r366872527
 
 

 ##
 File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileThread.java
 ##
 @@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.agent.core.profile;
+
+import org.apache.skywalking.apm.agent.core.boot.ServiceManager;
+import org.apache.skywalking.apm.agent.core.logging.api.ILog;
+import org.apache.skywalking.apm.agent.core.logging.api.LogManager;
+
+/**
+ * Profile task process thread, dump segment executing thread stack.
+ *
+ * @author MrPro
+ */
+public class ProfileThread implements Runnable {
+
+private static final ILog logger = 
LogManager.getLogger(ProfileThread.class);
+
+// profiling task context
+private final ProfileTaskExecutionContext taskExecutionContext;
+
+private final ProfileTaskExecutionService profileTaskExecutionService;
+private final ProfileTaskChannelService profileTaskChannelService;
+
+public ProfileThread(ProfileTaskExecutionContext taskExecutionContext) {
+this.taskExecutionContext = taskExecutionContext;
+profileTaskExecutionService = 
ServiceManager.INSTANCE.findService(ProfileTaskExecutionService.class);
+profileTaskChannelService = 
ServiceManager.INSTANCE.findService(ProfileTaskChannelService.class);
+}
+
+@Override
+public void run() {
+
+try {
+profiling(taskExecutionContext);
+} catch (InterruptedException e) {
+// ignore interrupted
+// means current task has stopped
 
 Review comment:
   Is this for shutdown process only?


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services