[
https://issues.apache.org/jira/browse/BEAM-9167?focusedWorklogId=375358&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-375358
]
ASF GitHub Bot logged work on BEAM-9167:
----------------------------------------
Author: ASF GitHub Bot
Created on: 22/Jan/20 01:44
Start Date: 22/Jan/20 01:44
Worklog Time Spent: 10m
Work Description: lostluck commented on pull request #10654: [BEAM-9167]
Reduce Go SDK metric overhead
URL: https://github.com/apache/beam/pull/10654
This PR dramatically reduces the overhead of metrics in the Go SDK.
A contemporary side by side comparison of the benchmark in the metrics
package on my current machine:
benchmark old ns/op new ns/op
delta
BenchmarkMetrics/counter_inplace-12 585 249
-57.44%
BenchmarkMetrics/distribution_inplace-12 622 270
-56.59%
BenchmarkMetrics/gauge_inplace-12 812 311
-61.70%
BenchmarkMetrics/counter_predeclared-12 227 15.8
-93.04%
BenchmarkMetrics/distribution_predeclared-12 282 24.0
-91.49%
BenchmarkMetrics/gauge_predeclared-12 389 63.7
-83.62%
benchmark old allocs new allocs
delta
BenchmarkMetrics/counter_inplace-12 4 1
-75.00%
BenchmarkMetrics/distribution_inplace-12 4 1
-75.00%
BenchmarkMetrics/gauge_inplace-12 4 1
-75.00%
BenchmarkMetrics/counter_predeclared-12 3 0
-100.00%
BenchmarkMetrics/distribution_predeclared-12 3 0
-100.00%
BenchmarkMetrics/gauge_predeclared-12 3 0
-100.00%
benchmark old bytes new bytes
delta
BenchmarkMetrics/counter_inplace-12 160 48
-70.00%
BenchmarkMetrics/distribution_inplace-12 192 48
-75.00%
BenchmarkMetrics/gauge_inplace-12 192 48
-75.00%
BenchmarkMetrics/counter_predeclared-12 48 0
-100.00%
BenchmarkMetrics/distribution_predeclared-12 80 0
-100.00%
BenchmarkMetrics/gauge_predeclared-12 80 0
-100.00%
In particular this PR moves away from a global datastore for all metrics
towards a perBundle based countersets. This allows for the removal of the per
layer locks and the global lock that needed to be checked since all bundles had
to check the same datastore. Now they only store a metric cell in the global
store on first creation (still stored per bundle and per ptransform).
A subsequent change will remove the global store altogether in favour of
better exposing the metrics per bundle, and allowing a callback visitor to
thread-safely access the data inside each metric. This will also permit
removing the dependency on the protos from the package, which was a mistake I
made when I first wrote the package.
Further, Counters now use atomic operations rather than locks, which
additional speeds them up vs the previous mutex approach.
Counter "names" are hashed ahead of time and the hash value cached in the
proxy to increase the speed of subsequent lookups using the same proxy object.
This does make the proxies unsafe to use concurrently within the same bundle
prior to first use, but this matches the general rule of Beam runners managing
the concurrency for efficient processing, and that framework constructs are not
safe for concurrent use by user code, without user managed locks.
As an exploration, I did try using sync.Map to avoid the above restriction,
but the overhead for the additional interface wraping and unwraping was
significant enough that this approach was worthwhile.
This may be worth revisiting if Go gains Generics, as that would probably
avoid this cost.
------------------------
Thank you for your contribution! Follow this checklist to help us
incorporate your contribution quickly and easily:
- [ ] [**Choose
reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and
mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA
issue, if applicable. This will automatically link the pull request to the
issue.
- [ ] If this contribution is large, please file an Apache [Individual
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more
tips on [how to make review process
smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
--- | --- | --- | --- | --- | --- | --- | ---
Go | [](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/)
| --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/)
| --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
Java | [](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/)
| [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/)
| [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)
| [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/)
| [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/)
| [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/)
| [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
Python | [](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)
| --- | [](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/)
| [](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/)
| --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
XLang | --- | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/)
| --- | --- | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/)
| [](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)
| [](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/)
| [](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/)
| --- | ---
See
[.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md)
for trigger phrase, status and link of all Jenkins jobs.
----------------------------------------------------------------
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:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 375358)
Remaining Estimate: 0h
Time Spent: 10m
> Reduce overhead of Go SDK side metrics
> --------------------------------------
>
> Key: BEAM-9167
> URL: https://issues.apache.org/jira/browse/BEAM-9167
> Project: Beam
> Issue Type: Improvement
> Components: sdk-go
> Reporter: Robert Burke
> Assignee: Robert Burke
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Locking overhead due to the global store and local caches of SDK counter data
> can dominate certain workloads, which means we can do better.
> Instead of having a global store of metrics data to extract counters, we
> should use per ptransform (or per bundle) counter sets, which would avoid
> requiring locking per counter operation. The main detriment compared to the
> current implementation is that a user would need to add their own locking if
> they were to spawn multiple goroutines to process a Bundle's work in a DoFn.
> Given that self multithreaded DoFns aren't recommended/safe in Java, largely
> impossible in Python, and the other beam Go SDK provided constructs (like
> Iterators and Emitters) are not thread safe, this is a small concern,
> provided the documentation is clear on this.
> Removing the locking and switching to atomic ops reduces the overhead
> significantly in example jobs and in the benchmarks.
> A second part of this change should be to move the exec package to manage
> it's own per bundle state, rather than relying on a global datastore to
> extract the per bundle,per ptransform values.
> Related: https://issues.apache.org/jira/browse/BEAM-6541
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
