Rémy,
This is a good one. Nice catch.
-chris
On 9/27/23 09:59, r...@apache.org wrote:
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 28e1010655 Refactor to remove syncs for upgraded connections
statistics
28e1010655 is described below
commit 28e1010655d2952a94550fa1156eed5f280f2620
Author: remm <r...@apache.org>
AuthorDate: Wed Sep 27 15:59:17 2023 +0200
Refactor to remove syncs for upgraded connections statistics
Found with coverity, while looking at UpgradeInfo.
---
.../coyote/http11/upgrade/UpgradeGroupInfo.java | 63 ++++++++++++----------
webapps/docs/changelog.xml | 4 ++
2 files changed, 38 insertions(+), 29 deletions(-)
diff --git a/java/org/apache/coyote/http11/upgrade/UpgradeGroupInfo.java
b/java/org/apache/coyote/http11/upgrade/UpgradeGroupInfo.java
index 7d729767d8..dc44062461 100644
--- a/java/org/apache/coyote/http11/upgrade/UpgradeGroupInfo.java
+++ b/java/org/apache/coyote/http11/upgrade/UpgradeGroupInfo.java
@@ -16,8 +16,9 @@
*/
package org.apache.coyote.http11.upgrade;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.LongAdder;
import org.apache.tomcat.util.modeler.BaseModelMBean;
@@ -26,85 +27,89 @@ import org.apache.tomcat.util.modeler.BaseModelMBean;
*/
public class UpgradeGroupInfo extends BaseModelMBean {
- private final List<UpgradeInfo> upgradeInfos = new ArrayList<>();
+ private final Set<UpgradeInfo> upgradeInfos = (new
ConcurrentHashMap<UpgradeInfo,Boolean>()).keySet(Boolean.TRUE);
- private long deadBytesReceived = 0;
- private long deadBytesSent = 0;
- private long deadMsgsReceived = 0;
- private long deadMsgsSent = 0;
+ private LongAdder deadBytesReceived = new LongAdder();
+ private LongAdder deadBytesSent = new LongAdder();
+ private LongAdder deadMsgsReceived = new LongAdder();
+ private LongAdder deadMsgsSent = new LongAdder();
- public synchronized void addUpgradeInfo(UpgradeInfo ui) {
+ public void addUpgradeInfo(UpgradeInfo ui) {
upgradeInfos.add(ui);
}
- public synchronized void removeUpgradeInfo(UpgradeInfo ui) {
+ public void removeUpgradeInfo(UpgradeInfo ui) {
if (ui != null) {
- deadBytesReceived += ui.getBytesReceived();
- deadBytesSent += ui.getBytesSent();
- deadMsgsReceived += ui.getMsgsReceived();
- deadMsgsSent += ui.getMsgsSent();
+ deadBytesReceived.add(ui.getBytesReceived());
+ deadBytesSent.add(ui.getBytesSent());
+ deadMsgsReceived.add(ui.getMsgsReceived());
+ deadMsgsSent.add(ui.getMsgsSent());
upgradeInfos.remove(ui);
}
}
- public synchronized long getBytesReceived() {
- long bytes = deadBytesReceived;
+ public long getBytesReceived() {
+ long bytes = deadBytesReceived.longValue();
for (UpgradeInfo ui : upgradeInfos) {
bytes += ui.getBytesReceived();
}
return bytes;
}
- public synchronized void setBytesReceived(long bytesReceived) {
- deadBytesReceived = bytesReceived;
+ public void setBytesReceived(long bytesReceived) {
+ deadBytesReceived.reset();
+ deadBytesReceived.add(bytesReceived);
for (UpgradeInfo ui : upgradeInfos) {
ui.setBytesReceived(bytesReceived);
}
}
- public synchronized long getBytesSent() {
- long bytes = deadBytesSent;
+ public long getBytesSent() {
+ long bytes = deadBytesSent.longValue();
for (UpgradeInfo ui : upgradeInfos) {
bytes += ui.getBytesSent();
}
return bytes;
}
- public synchronized void setBytesSent(long bytesSent) {
- deadBytesSent = bytesSent;
+ public void setBytesSent(long bytesSent) {
+ deadBytesSent.reset();
+ deadBytesSent.add(bytesSent);
for (UpgradeInfo ui : upgradeInfos) {
ui.setBytesSent(bytesSent);
}
}
- public synchronized long getMsgsReceived() {
- long msgs = deadMsgsReceived;
+ public long getMsgsReceived() {
+ long msgs = deadMsgsReceived.longValue();
for (UpgradeInfo ui : upgradeInfos) {
msgs += ui.getMsgsReceived();
}
return msgs;
}
- public synchronized void setMsgsReceived(long msgsReceived) {
- deadMsgsReceived = msgsReceived;
+ public void setMsgsReceived(long msgsReceived) {
+ deadMsgsReceived.reset();
+ deadMsgsReceived.add(msgsReceived);
for (UpgradeInfo ui : upgradeInfos) {
ui.setMsgsReceived(msgsReceived);
}
}
- public synchronized long getMsgsSent() {
- long msgs = deadMsgsSent;
+ public long getMsgsSent() {
+ long msgs = deadMsgsSent.longValue();
for (UpgradeInfo ui : upgradeInfos) {
msgs += ui.getMsgsSent();
}
return msgs;
}
- public synchronized void setMsgsSent(long msgsSent) {
- deadMsgsSent = msgsSent;
+ public void setMsgsSent(long msgsSent) {
+ deadMsgsSent.reset();
+ deadMsgsSent.add(msgsSent);
for (UpgradeInfo ui : upgradeInfos) {
ui.setMsgsSent(msgsSent);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index aa98ce6c99..424f2bfb1d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -156,6 +156,10 @@
<fix>
Avoid rare thread safety issue accessing message digest map. (remm)
</fix>
+ <fix>
+ Improve statistics collection for upgraded connections under load.
+ (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
