You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "rtadepalli (via GitHub)" <gi...@apache.org> on 2023/06/17 22:33:00 UTC
[GitHub] [arrow] rtadepalli opened a new pull request, #36152: GH-36151: Add `volatile` declaration to `keyPosition` in `ParallelSearcher`
rtadepalli opened a new pull request, #36152:
URL: https://github.com/apache/arrow/pull/36152
### Rationale for this change
Multiple threads can read/write to the `keyPosition` variable. Thus, it should be declared `volatile` so that all threads have a consistent view of the value of this variable.
### What changes are included in this PR?
This PR just marks an instance member `volatile`.
### Are these changes tested?
Existing tests should suffice as no functionality is being changed.
### Are there any user-facing changes?
No, there are no user facing changes. There are no changes to the public API either.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm merged pull request #36152: GH-36151: [Java] Add `volatile` declaration to `keyPosition` in `ParallelSearcher`
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #36152:
URL: https://github.com/apache/arrow/pull/36152
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #36152: GH-36151: [Java] Add `volatile` declaration to `keyPosition` in `ParallelSearcher`
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #36152:
URL: https://github.com/apache/arrow/pull/36152#issuecomment-1597911698
Conbench analyzed the 6 benchmark runs on commit `9589338e`.
There was 1 benchmark result indicating a performance regression:
- Commit Run on `ursa-thinkcentre-m75q` at [2023-06-19 14:37:31Z](http://conbench.ursa.dev/compare/runs/af40cafbc20c4534be7eff0707bdb47a...a14739e91b774176a935f6a0fc9e0c48/)
- [params=32768/1, source=cpp-micro, suite=arrow-compare-benchmark](http://conbench.ursa.dev/compare/benchmarks/0648fd8d8252798d800032bc2316add2...06490691199c7aee800040a5e6f6d2fb)
The [full Conbench report](https://github.com/apache/arrow/runs/14388679217) has more details.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #36152: GH-36151: [Java] Add `volatile` declaration to `keyPosition` in `ParallelSearcher`
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #36152:
URL: https://github.com/apache/arrow/pull/36152#issuecomment-1597148991
Ah, Java's volatile is stronger than C/C++'s.
The algorithm as written is still nondeterministic, but I suppose that may be intentional.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rtadepalli commented on pull request #36152: GH-36151: [Java] Add `volatile` declaration to `keyPosition` in `ParallelSearcher`
Posted by "rtadepalli (via GitHub)" <gi...@apache.org>.
rtadepalli commented on PR #36152:
URL: https://github.com/apache/arrow/pull/36152#issuecomment-1597109933
`keyPosition` is only being read and written to, both of which should be one instruction each. Per my understanding, I don't think we need `AtomicInteger` here; `volatile` should suffice. Please let me know if I am missing something.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org