You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "lhotari (via GitHub)" <gi...@apache.org> on 2023/05/29 11:49:55 UTC

[GitHub] [pulsar] lhotari opened a new pull request, #20428: [improve][broker] Remove uneffective solution for reducing GC pressure

lhotari opened a new pull request, #20428:
URL: https://github.com/apache/pulsar/pull/20428

   ### Motivation
   
   The PublisherStatsImpl and ConsumerStatsImpl classes contain an uneffective solution for reducing GC pressure. 
   This code is confusing and doesn't help. Instead, ordinary fields are used.
   
   ### Modifications
   
   - remove the uneffective solution
   - add a solution for reducing unnecessary object creation by sharing the client source address and port String object across all consumer and publisher stats on a single connection.
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc` <!-- Your PR contains doc changes. -->
   - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->


-- 
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: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] tisonkun commented on a diff in pull request #20428: [improve][broker] Remove uneffective solution for reducing GC pressure

Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #20428:
URL: https://github.com/apache/pulsar/pull/20428#discussion_r1211648296


##########
pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/PublisherStatsImpl.java:
##########
@@ -53,35 +53,13 @@ public class PublisherStatsImpl implements PublisherStats {
     public boolean supportsPartialProducer;
 
     /** Producer name. */
-    @JsonIgnore
-    private int producerNameOffset = -1;
-    @JsonIgnore
-    private int producerNameLength;
-
+    private String producerName;

Review Comment:
   How can these changes affect JSON serde code path? I don't know if we pass or persist these classes via JSON somewhere.
   
   If no, `@JsonIgnore` can be redundant. If so, adding new serialize field may affect especially this is a class under pulsar-common.



-- 
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: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] lhotari commented on pull request #20428: [improve][broker] Remove uneffective solution for reducing GC pressure

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #20428:
URL: https://github.com/apache/pulsar/pull/20428#issuecomment-1567038127

   /pulsarbot rerun-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] lhotari commented on a diff in pull request #20428: [improve][broker] Remove uneffective solution for reducing GC pressure

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #20428:
URL: https://github.com/apache/pulsar/pull/20428#discussion_r1211709176


##########
pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/PublisherStatsImpl.java:
##########
@@ -53,35 +53,13 @@ public class PublisherStatsImpl implements PublisherStats {
     public boolean supportsPartialProducer;
 
     /** Producer name. */
-    @JsonIgnore
-    private int producerNameOffset = -1;
-    @JsonIgnore
-    private int producerNameLength;
-
+    private String producerName;

Review Comment:
   There is no change in JSON serialization and deserialization from an external viewpoint.



-- 
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: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] lhotari commented on a diff in pull request #20428: [improve][broker] Remove uneffective solution for reducing GC pressure

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #20428:
URL: https://github.com/apache/pulsar/pull/20428#discussion_r1211706779


##########
pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/PublisherStatsImpl.java:
##########
@@ -53,35 +53,13 @@ public class PublisherStatsImpl implements PublisherStats {
     public boolean supportsPartialProducer;
 
     /** Producer name. */
-    @JsonIgnore
-    private int producerNameOffset = -1;
-    @JsonIgnore
-    private int producerNameLength;
-
+    private String producerName;

Review Comment:
   The former `@JsonIgnore` annotations were needed because of the extra ordinary solution to store String values. They aren't needed anymore.



-- 
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: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] lhotari merged pull request #20428: [improve][broker] Remove uneffective solution for reducing GC pressure

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari merged PR #20428:
URL: https://github.com/apache/pulsar/pull/20428


-- 
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: commits-unsubscribe@pulsar.apache.org

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