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 2021/05/16 17:11:56 UTC

[GitHub] [skywalking-nginx-lua] yxudong opened a new pull request #83: use table.concat to optimize performance

yxudong opened a new pull request #83:
URL: https://github.com/apache/skywalking-nginx-lua/pull/83


   use table.concat to optimize performance of string concatenation
   
   


-- 
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



[GitHub] [skywalking-nginx-lua] wu-sheng commented on pull request #83: use table.concat to optimize performance

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #83:
URL: https://github.com/apache/skywalking-nginx-lua/pull/83#issuecomment-841897791


   FYI @membphis 


-- 
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



[GitHub] [skywalking-nginx-lua] wu-sheng commented on pull request #83: use table.concat to optimize performance

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #83:
URL: https://github.com/apache/skywalking-nginx-lua/pull/83#issuecomment-841916561


   Seems testing breaks. Please test.


-- 
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



[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull request #83: use table.concat to optimize performance

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #83:
URL: https://github.com/apache/skywalking-nginx-lua/pull/83#discussion_r633596368



##########
File path: lib/skywalking/segment_ref.lua
##########
@@ -59,14 +59,16 @@ end
 
 -- Return string to represent this ref.
 function _M.serialize(ref)
-    local encodedRef = '1'
-            .. '-' .. encode_base64(ref.trace_id)
-            .. '-' .. encode_base64(ref.segment_id)
-            .. '-' .. ref.span_id
-            .. '-' .. encode_base64(ref.parent_service)
-            .. '-' .. encode_base64(ref.parent_service_instance)
-            .. '-' .. encode_base64(ref.parent_endpoint)
-            .. '-' .. encode_base64(ref.address_used_at_client)
+    local encodedRef = table.concat({

Review comment:
       Thanks for confirming.




-- 
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



[GitHub] [skywalking-nginx-lua] yxudong commented on pull request #83: use table.concat to optimize performance

Posted by GitBox <gi...@apache.org>.
yxudong commented on pull request #83:
URL: https://github.com/apache/skywalking-nginx-lua/pull/83#issuecomment-842010794


   Sorry, I've fixed 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:
users@infra.apache.org



[GitHub] [skywalking-nginx-lua] wu-sheng closed pull request #83: use table.concat to optimize performance

Posted by GitBox <gi...@apache.org>.
wu-sheng closed pull request #83:
URL: https://github.com/apache/skywalking-nginx-lua/pull/83


   


-- 
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



[GitHub] [skywalking-nginx-lua] wu-sheng closed pull request #83: use table.concat to optimize performance

Posted by GitBox <gi...@apache.org>.
wu-sheng closed pull request #83:
URL: https://github.com/apache/skywalking-nginx-lua/pull/83


   


-- 
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



[GitHub] [skywalking-nginx-lua] yxudong commented on pull request #83: use table.concat to optimize performance

Posted by GitBox <gi...@apache.org>.
yxudong commented on pull request #83:
URL: https://github.com/apache/skywalking-nginx-lua/pull/83#issuecomment-842010794


   Sorry, I've fixed 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:
users@infra.apache.org



[GitHub] [skywalking-nginx-lua] wu-sheng commented on pull request #83: use table.concat to optimize performance

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #83:
URL: https://github.com/apache/skywalking-nginx-lua/pull/83#issuecomment-841897791






-- 
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



[GitHub] [skywalking-nginx-lua] membphis commented on a change in pull request #83: use table.concat to optimize performance

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #83:
URL: https://github.com/apache/skywalking-nginx-lua/pull/83#discussion_r633595423



##########
File path: lib/skywalking/segment_ref.lua
##########
@@ -59,14 +59,16 @@ end
 
 -- Return string to represent this ref.
 function _M.serialize(ref)
-    local encodedRef = '1'
-            .. '-' .. encode_base64(ref.trace_id)
-            .. '-' .. encode_base64(ref.segment_id)
-            .. '-' .. ref.span_id
-            .. '-' .. encode_base64(ref.parent_service)
-            .. '-' .. encode_base64(ref.parent_service_instance)
-            .. '-' .. encode_base64(ref.parent_endpoint)
-            .. '-' .. encode_base64(ref.address_used_at_client)
+    local encodedRef = table.concat({

Review comment:
       Performance in this way will be lower. 
   
   `table.concat` is only needed when the number of elements of concat is not fixed.




-- 
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



[GitHub] [skywalking-nginx-lua] membphis commented on a change in pull request #83: use table.concat to optimize performance

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #83:
URL: https://github.com/apache/skywalking-nginx-lua/pull/83#discussion_r633595423



##########
File path: lib/skywalking/segment_ref.lua
##########
@@ -59,14 +59,16 @@ end
 
 -- Return string to represent this ref.
 function _M.serialize(ref)
-    local encodedRef = '1'
-            .. '-' .. encode_base64(ref.trace_id)
-            .. '-' .. encode_base64(ref.segment_id)
-            .. '-' .. ref.span_id
-            .. '-' .. encode_base64(ref.parent_service)
-            .. '-' .. encode_base64(ref.parent_service_instance)
-            .. '-' .. encode_base64(ref.parent_endpoint)
-            .. '-' .. encode_base64(ref.address_used_at_client)
+    local encodedRef = table.concat({

Review comment:
       Performance in this way will be lower. 
   
   `table.concat` is only needed when the number of elements of concat is not fixed.




-- 
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



[GitHub] [skywalking-nginx-lua] wu-sheng commented on a change in pull request #83: use table.concat to optimize performance

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #83:
URL: https://github.com/apache/skywalking-nginx-lua/pull/83#discussion_r633596368



##########
File path: lib/skywalking/segment_ref.lua
##########
@@ -59,14 +59,16 @@ end
 
 -- Return string to represent this ref.
 function _M.serialize(ref)
-    local encodedRef = '1'
-            .. '-' .. encode_base64(ref.trace_id)
-            .. '-' .. encode_base64(ref.segment_id)
-            .. '-' .. ref.span_id
-            .. '-' .. encode_base64(ref.parent_service)
-            .. '-' .. encode_base64(ref.parent_service_instance)
-            .. '-' .. encode_base64(ref.parent_endpoint)
-            .. '-' .. encode_base64(ref.address_used_at_client)
+    local encodedRef = table.concat({

Review comment:
       Thanks for confirming.




-- 
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