[
https://issues.apache.org/jira/browse/BEAM-14166?focusedWorklogId=769334&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769334
]
ASF GitHub Bot logged work on BEAM-14166:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 11/May/22 22:07
Start Date: 11/May/22 22:07
Worklog Time Spent: 10m
Work Description: reuvenlax commented on PR #17172:
URL: https://github.com/apache/beam/pull/17172#issuecomment-1124338991
Let me take another look. IIRC I had some concerns, but it's possible that
the extensive benchmarking addressed those.
On Wed, May 11, 2022 at 2:58 PM Brian Hulette ***@***.***>
wrote:
> ***@***.**** approved this pull request.
>
> Thanks @mosche <https://github.com/mosche> for the extensive
> benchmarking, and apologies for the atrocious review latency!
>
> I'm ok with this if it passes current tests, given the extensive
> benchmarking. A couple of things:
>
> - I'd still like to get some final feedback on the approach from
> @reuvenlax <https://github.com/reuvenlax>, but I acn go ahead and
> merge next Monday if that doesn't happen this week.
> - It could be interesting to run your benchmarks continuously to 1)
> monitor for regressions, and 2) give us a convenient way to test other
> potential improvements. Maybe file a jira for that if you don't have
time
> now?
>
> ------------------------------
>
> In
>
sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/FieldValueGetter.java
> <https://github.com/apache/beam/pull/17172#discussion_r870780914>:
>
> > @@ -33,5 +33,9 @@
> @Nullable
> ValueT get(ObjectT object);
>
> + default @Nullable Object getRaw(ObjectT object) {
>
> nit: maybe getUntyped is a more appropriate name here?
>
> Also could you clarify why we need this?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/beam/pull/17172#pullrequestreview-970037354>,
> or unsubscribe
>
<https://github.com/notifications/unsubscribe-auth/AFAYJVOBUJMEVNFJNON7Y7DVJQUOVANCNFSM5RRHFBKQ>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
Issue Time Tracking
-------------------
Worklog Id: (was: 769334)
Time Spent: 3h (was: 2h 50m)
> Improvements to RowWithGetter
> -----------------------------
>
> Key: BEAM-14166
> URL: https://issues.apache.org/jira/browse/BEAM-14166
> Project: Beam
> Issue Type: Improvement
> Components: sdk-java-core
> Reporter: Moritz Mack
> Assignee: Moritz Mack
> Priority: P2
> Time Spent: 3h
> Remaining Estimate: 0h
>
> Various improvements to getValue(fieldIdx) in RowWithGetters mentioned
> [here|[https://github.com/apache/beam/pull/16947#discussion_r833602836]:]
> * Minimize memory overhead of cache using either a index lookup (array) or a
> single hash map if number of fields exceeds the initial hashmap capacity
> * The cache should be checked before calling a getter to avoid any
> potentially unnecessary conversion in the getter itself.
> * [Nested
> rows|https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/values/RowWithGetters.java#L111]
> should be cached, otherwise the cache of such nested rows can't be leveraged.
> * Handling of collections / maps / iterables can be significantly improved
> by simply skipping the transform in all cases where {{getValue}} for members
> is the [identity
> transform|https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/values/RowWithGetters.java#L142].
>
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
