You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/02/09 03:31:21 UTC

[GitHub] [skywalking] mrproliu opened a new pull request #4332: Change profile stack element data structure; 

mrproliu opened a new pull request #4332: Change profile stack element data structure;
URL: https://github.com/apache/skywalking/pull/4332
 
 
   Please answer these questions before submitting pull request
   
   - Why submit this pull request?
   - [x] Bug fix
   - [ ] New feature provided
   - [ ] Improve performance
   
   - Related issues
   #4104 
   ___
   ### Bug fix
   - GraphQL cannot support self reference, cannot build query request.
   
   - How to fix?
   Change profile stack element using single level list, use `id` and `parentId` to reference parent element.
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4332: Change profile stack element data structure

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4332: Change profile stack element data structure
URL: https://github.com/apache/skywalking/pull/4332#discussion_r376758840
 
 

 ##########
 File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/profile/analyze/ProfileStackNode.java
 ##########
 @@ -163,21 +165,28 @@ public ProfileStackElement buildAnalyzeResult() {
             ProfileStackElement respElement = mergingPair.key;
 
             // generate children node and add to stack and all node mapping
-            respElement.setChildren(mergingPair.value.children.stream().map(c -> {
-                ProfileStackElement element = c.buildElement();
-                Pair<ProfileStackElement, ProfileStackNode> pair = new Pair<>(element, c);
+            for (ProfileStackNode children : mergingPair.value.children) {
+                ProfileStackElement element = children.buildElement(idGenerator++);
+                element.setParentId(respElement.getId());
+
+                Pair<ProfileStackElement, ProfileStackNode> pair = new Pair<>(element, children);
                 stack.add(pair);
                 nodeMapping.add(pair);
-
-                return element;
-            }).collect(Collectors.toList()));
+            }
         }
 
         // calculate durations
         nodeMapping.parallelStream().forEach(t -> t.value.calculateDuration(t.key));
         nodeMapping.parallelStream().forEach(t -> t.value.calculateDurationExcludeChild(t.key));
 
-        return root;
+        ProfileStackTree tree = new ProfileStackTree();
+        ArrayList<ProfileStackElement> elements = new ArrayList<>(nodeMapping.size());
+        for (Pair<ProfileStackElement, ProfileStackNode> pair : nodeMapping) {
+            elements.add(pair.key);
+        }
+        tree.setElements(elements);
 
 Review comment:
   Use codes like `nodeMapping.forEach(node-> tree.getElements().add(node.value));`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io commented on issue #4332: Change profile stack element data structure

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4332: Change profile stack element data structure
URL: https://github.com/apache/skywalking/pull/4332#issuecomment-583804147
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=h1) Report
   > Merging [#4332](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/f29fc4bcb32060003b9b4771b242018c25f4588d?src=pr&el=desc) will **decrease** coverage by `<.01%`.
   > The diff coverage is `92.85%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4332/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##           master   #4332      +/-   ##
   =========================================
   - Coverage   27.01%     27%   -0.01%     
   =========================================
     Files        1175    1175              
     Lines       25672   25675       +3     
     Branches     3663    3664       +1     
   =========================================
   - Hits         6935    6934       -1     
   - Misses      18127   18128       +1     
   - Partials      610     613       +3
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [.../core/profile/analyze/ProfileAnalyzeCollector.java](https://codecov.io/gh/apache/skywalking/pull/4332/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcHJvZmlsZS9hbmFseXplL1Byb2ZpbGVBbmFseXplQ29sbGVjdG9yLmphdmE=) | `100% <ø> (ø)` | :arrow_up: |
   | [...lking/oap/query/graphql/resolver/ProfileQuery.java](https://codecov.io/gh/apache/skywalking/pull/4332/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcXVlcnktcGx1Z2luL3F1ZXJ5LWdyYXBocWwtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9xdWVyeS9ncmFwaHFsL3Jlc29sdmVyL1Byb2ZpbGVRdWVyeS5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...p/server/core/profile/analyze/ProfileAnalyzer.java](https://codecov.io/gh/apache/skywalking/pull/4332/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcHJvZmlsZS9hbmFseXplL1Byb2ZpbGVBbmFseXplci5qYXZh) | `77.77% <100%> (ø)` | :arrow_up: |
   | [.../server/core/profile/analyze/ProfileStackNode.java](https://codecov.io/gh/apache/skywalking/pull/4332/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcHJvZmlsZS9hbmFseXplL1Byb2ZpbGVTdGFja05vZGUuamF2YQ==) | `90.38% <100%> (+0.28%)` | :arrow_up: |
   | [...king/apm/agent/core/remote/GRPCChannelManager.java](https://codecov.io/gh/apache/skywalking/pull/4332/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsTWFuYWdlci5qYXZh) | `70.51% <0%> (-5.13%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=footer). Last update [f29fc4b...52125d5](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng merged pull request #4332: Change profile stack element data structure

Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #4332: Change profile stack element data structure
URL: https://github.com/apache/skywalking/pull/4332
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4332: Change profile stack element data structure

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4332: Change profile stack element data structure
URL: https://github.com/apache/skywalking/pull/4332#issuecomment-583804147
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=h1) Report
   > Merging [#4332](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/f29fc4bcb32060003b9b4771b242018c25f4588d?src=pr&el=desc) will **increase** coverage by `0.01%`.
   > The diff coverage is `93.75%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4332/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4332      +/-   ##
   ==========================================
   + Coverage   27.01%   27.02%   +0.01%     
   ==========================================
     Files        1175     1176       +1     
     Lines       25672    25677       +5     
     Branches     3663     3664       +1     
   ==========================================
   + Hits         6935     6940       +5     
     Misses      18127    18127              
     Partials      610      610
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [.../core/profile/analyze/ProfileAnalyzeCollector.java](https://codecov.io/gh/apache/skywalking/pull/4332/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcHJvZmlsZS9hbmFseXplL1Byb2ZpbGVBbmFseXplQ29sbGVjdG9yLmphdmE=) | `100% <ø> (ø)` | :arrow_up: |
   | [...lking/oap/query/graphql/resolver/ProfileQuery.java](https://codecov.io/gh/apache/skywalking/pull/4332/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcXVlcnktcGx1Z2luL3F1ZXJ5LWdyYXBocWwtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9xdWVyeS9ncmFwaHFsL3Jlc29sdmVyL1Byb2ZpbGVRdWVyeS5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...p/server/core/profile/analyze/ProfileAnalyzer.java](https://codecov.io/gh/apache/skywalking/pull/4332/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcHJvZmlsZS9hbmFseXplL1Byb2ZpbGVBbmFseXplci5qYXZh) | `77.77% <100%> (ø)` | :arrow_up: |
   | [...oap/server/core/query/entity/ProfileStackTree.java](https://codecov.io/gh/apache/skywalking/pull/4332/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvZW50aXR5L1Byb2ZpbGVTdGFja1RyZWUuamF2YQ==) | `100% <100%> (ø)` | |
   | [.../server/core/profile/analyze/ProfileStackNode.java](https://codecov.io/gh/apache/skywalking/pull/4332/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcHJvZmlsZS9hbmFseXplL1Byb2ZpbGVTdGFja05vZGUuamF2YQ==) | `90.38% <100%> (+0.28%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=footer). Last update [f29fc4b...83f8f67](https://codecov.io/gh/apache/skywalking/pull/4332?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4332: Change profile stack element data structure

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4332: Change profile stack element data structure
URL: https://github.com/apache/skywalking/pull/4332#discussion_r376758126
 
 

 ##########
 File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/profile/analyze/ProfileStackNode.java
 ##########
 @@ -163,21 +166,24 @@ public ProfileStackElement buildAnalyzeResult() {
             ProfileStackElement respElement = mergingPair.key;
 
             // generate children node and add to stack and all node mapping
-            respElement.setChildren(mergingPair.value.children.stream().map(c -> {
-                ProfileStackElement element = c.buildElement();
-                Pair<ProfileStackElement, ProfileStackNode> pair = new Pair<>(element, c);
+            for (ProfileStackNode children : mergingPair.value.children) {
+                ProfileStackElement element = children.buildElement(idGenerator++);
+                element.setParentId(respElement.getId());
+
+                Pair<ProfileStackElement, ProfileStackNode> pair = new Pair<>(element, children);
                 stack.add(pair);
                 nodeMapping.add(pair);
-
-                return element;
-            }).collect(Collectors.toList()));
+            }
         }
 
         // calculate durations
         nodeMapping.parallelStream().forEach(t -> t.value.calculateDuration(t.key));
         nodeMapping.parallelStream().forEach(t -> t.value.calculateDurationExcludeChild(t.key));
 
-        return root;
+        ProfileStackTree tree = new ProfileStackTree();
+        tree.setElements(nodeMapping.stream().map(p -> p.key).collect(Collectors.toList()));
 
 Review comment:
   This could be replaced by simplier `foreach`.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services