[
https://issues.apache.org/jira/browse/GEODE-2141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15706175#comment-15706175
]
ASF GitHub Bot commented on GEODE-2141:
---------------------------------------
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/299#discussion_r90077848
--- Diff:
geode-core/src/main/java/org/apache/geode/internal/statistics/StatMonitorHandler.java
---
@@ -50,36 +50,26 @@ public StatMonitorHandler() {
/** Adds a monitor which will be notified of samples */
public boolean addMonitor(StatisticsMonitor monitor) {
- synchronized (this) {
- boolean added = false;
- List<StatisticsMonitor> oldMonitors = this.monitors;
- if (!oldMonitors.contains(monitor)) {
- List<StatisticsMonitor> newMonitors = new
ArrayList<StatisticsMonitor>(oldMonitors);
- added = newMonitors.add(monitor);
- this.monitors = Collections.unmodifiableList(newMonitors);
- }
- if (!this.monitors.isEmpty()) {
- startNotifier_IfEnabledAndNotRunning();
- }
- return added;
+ boolean added = false;
+ if (!this.monitors.contains(monitor)) {
+ added = this.monitors.add(monitor);
+ }
+ if (!this.monitors.isEmpty()) {
+ startNotifier_IfEnabledAndNotRunning();
}
+ return added;
}
/** Removes a monitor that will no longer be used */
public boolean removeMonitor(StatisticsMonitor monitor) {
- synchronized (this) {
- boolean removed = false;
- List<StatisticsMonitor> oldMonitors = this.monitors;
- if (oldMonitors.contains(monitor)) {
- List<StatisticsMonitor> newMonitors = new
ArrayList<StatisticsMonitor>(oldMonitors);
- removed = newMonitors.remove(monitor);
- this.monitors = Collections.unmodifiableList(newMonitors);
- }
- if (this.monitors.isEmpty()) {
- stopNotifier_IfEnabledAndRunning();
- }
- return removed;
+ boolean removed = false;
--- End diff --
Same issue here - it's not safe to remove the synchronization.
> StatisticsMonitor and StatMonitorHandler should use ConcurrentHashSet to
> store monitors, listeners and statisticIds
> -------------------------------------------------------------------------------------------------------------------
>
> Key: GEODE-2141
> URL: https://issues.apache.org/jira/browse/GEODE-2141
> Project: Geode
> Issue Type: Improvement
> Reporter: Avinash Dongre
> Assignee: Avinash Dongre
>
> In StatisticsMonitor and StatMonitorHandler List is used to store monitors,
> listeners and statisticIds.
> We should be using ConcurrentHashSet for performance Reason.
> In my local testing When I replace the List to
> com.gemstone.gemfire.internal.concurrent.
> ConcurrentHashSet
> I see significant improvement in creating large number of region creation.
> ( from ~7hrs to ~26 minutes)
> More details about the same is on dev list.
> Refer : http://markmail.org/message/o7td3fczylx4uaet
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
