Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove creation of subrow from ReportQueryResult for batchfetch performance #2326

Closed

Conversation

Sheikah45
Copy link
Contributor

This is a follow up to #2303. After some additional testing I observed some performance degradation when using batchfetch in with a ReportQuery. The degraded performance occurred when the item being constructed did not have the first index in the row. In this case a subrow was created that contained only the fields after the item index. When a batchfetch policy was in use this also required performing the same action on all the rows returned by the query and setting them for the data results in the batch fetch policy for the item's mappings as they would be used in the ForeignReferenceMapping to extract the required keys.

This caused every subrow to have to be created for every item even if it has been previously done because the rows use identity based equality. After looking through the downstream uses of the subrow it is not clear why this trimming is necessary. For the downstream object building values are extracted using their corresponding field references rather than by index in the row.

For this reason I have removed the creation of the subrow altogether but am also curious what the original reason for the subrow was and if this may have other performance implications for downstream processing of the row.

@Sheikah45 Sheikah45 marked this pull request as ready for review December 14, 2024 15:29
@Sheikah45 Sheikah45 changed the title Remove creation of subRow from ReportQueryResult Remove creation of subrow from ReportQueryResult for batchfetch performance Dec 14, 2024
@Sheikah45 Sheikah45 marked this pull request as draft December 14, 2024 16:44
@Sheikah45 Sheikah45 closed this Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant