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

[ML] Transforms: Improve data grid memoization. #195394

Merged
merged 5 commits into from
Oct 11, 2024

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Oct 8, 2024

Summary

Part of #178606 and #151664.

  • Removes some unused code related to identifying populated index fields.
  • Changes useIndexData() to accept just one config options arg instead of individual args.
  • Improves data grid memoziation.

Improvements tested locally:

many_fields dataset (no timestamp)

  • main: ~22s and 10 data grid rerenders until many_fields data set loaded. The transform config dropdown are hardly usable and super slow, each edit causes 3 data grid rerenders.
  • This PR: ~4.5s and 7 data grid rerenders until many_fields data set loaded. The transform config dropdowns are a bit slow but usable!

kibana_sample_data_logs dataset (whole dataset in the past to test rerenders on load without data)

  • main: 5 rerenders.
  • This PR: 3 rerenders

Checklist

@walterra walterra force-pushed the ml-transform-data-grid-memoization branch from db57f75 to bf19735 Compare October 9, 2024 14:27
@walterra walterra marked this pull request as ready for review October 9, 2024 14:27
@walterra walterra requested a review from a team as a code owner October 9, 2024 14:27
@walterra walterra requested review from darnautov and rbrtj October 9, 2024 14:28
@walterra walterra added :ml release_note:skip Skip the PR/issue when compiling release notes Feature:Transforms ML transforms v9.0.0 v8.16.0 backport:version Backport to applied version labels labels Oct 9, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Contributor

@rbrtj rbrtj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Leaving this visibility:
When there are a lot of documents, we can see this error while navigating to some of the later pages of the results. However, it seems to work correctly even with the error.
It occurs on the main as well, so not really related to the PR

Screen.Recording.2024-10-11.at.11.59.27.mov

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 4.5MB 4.5MB +81.0B
transform 472.7KB 472.4KB -235.0B
total -154.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
@kbn/ml-data-grid 4 5 +1
transform 31 30 -1
total -0

Total ESLint disabled count

id before after diff
@kbn/ml-data-grid 4 5 +1
transform 35 34 -1
total -0

History

cc @walterra

@walterra walterra merged commit 869ceec into elastic:main Oct 11, 2024
27 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11297335705

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 11, 2024
## Summary

Part of elastic#178606 and elastic#151664.

- Removes some unused code related to identifying populated index
fields.
- Changes `useIndexData()` to accept just one config options arg instead
of individual args.
- Improves data grid memoziation.

Improvements tested locally:

#### `many_fields` dataset (no timestamp)

- `main`: `~22s` and 10 data grid rerenders until many_fields data set
loaded. The transform config dropdown are hardly usable and super slow,
each edit causes 3 data grid rerenders.
- This PR: `~4.5s` and 7 data grid rerenders until many_fields data set
loaded. The transform config dropdowns are a bit slow but usable!

#### `kibana_sample_data_logs` dataset (whole dataset in the past to
test rerenders on load without data)

- `main`: 5 rerenders.
- This PR: 3 rerenders

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 869ceec)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 11, 2024
)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ML] Transforms: Improve data grid memoization.
(#195394)](#195394)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Walter
Rafelsberger","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-11T18:18:11Z","message":"[ML]
Transforms: Improve data grid memoization. (#195394)\n\n##
Summary\r\n\r\nPart of #178606 and #151664.\r\n\r\n- Removes some unused
code related to identifying populated index\r\nfields.\r\n- Changes
`useIndexData()` to accept just one config options arg instead\r\nof
individual args.\r\n- Improves data grid
memoziation.\r\n\r\nImprovements tested locally:\r\n\r\n####
`many_fields` dataset (no timestamp)\r\n\r\n- `main`: `~22s` and 10 data
grid rerenders until many_fields data set\r\nloaded. The transform
config dropdown are hardly usable and super slow,\r\neach edit causes 3
data grid rerenders.\r\n- This PR: `~4.5s` and 7 data grid rerenders
until many_fields data set\r\nloaded. The transform config dropdowns are
a bit slow but usable!\r\n\r\n#### `kibana_sample_data_logs` dataset
(whole dataset in the past to\r\ntest rerenders on load without
data)\r\n\r\n- `main`: 5 rerenders.\r\n- This PR: 3 rerenders\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [x] This was checked for breaking
API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"869ceec5ca8a1156d077bb2a888a91ef73e30511","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":[":ml","release_note:skip","Feature:Transforms","v9.0.0","v8.16.0","backport:version"],"title":"[ML]
Transforms: Improve data grid
memoization.","number":195394,"url":"https://github.com/elastic/kibana/pull/195394","mergeCommit":{"message":"[ML]
Transforms: Improve data grid memoization. (#195394)\n\n##
Summary\r\n\r\nPart of #178606 and #151664.\r\n\r\n- Removes some unused
code related to identifying populated index\r\nfields.\r\n- Changes
`useIndexData()` to accept just one config options arg instead\r\nof
individual args.\r\n- Improves data grid
memoziation.\r\n\r\nImprovements tested locally:\r\n\r\n####
`many_fields` dataset (no timestamp)\r\n\r\n- `main`: `~22s` and 10 data
grid rerenders until many_fields data set\r\nloaded. The transform
config dropdown are hardly usable and super slow,\r\neach edit causes 3
data grid rerenders.\r\n- This PR: `~4.5s` and 7 data grid rerenders
until many_fields data set\r\nloaded. The transform config dropdowns are
a bit slow but usable!\r\n\r\n#### `kibana_sample_data_logs` dataset
(whole dataset in the past to\r\ntest rerenders on load without
data)\r\n\r\n- `main`: 5 rerenders.\r\n- This PR: 3 rerenders\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [x] This was checked for breaking
API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"869ceec5ca8a1156d077bb2a888a91ef73e30511"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195394","number":195394,"mergeCommit":{"message":"[ML]
Transforms: Improve data grid memoization. (#195394)\n\n##
Summary\r\n\r\nPart of #178606 and #151664.\r\n\r\n- Removes some unused
code related to identifying populated index\r\nfields.\r\n- Changes
`useIndexData()` to accept just one config options arg instead\r\nof
individual args.\r\n- Improves data grid
memoziation.\r\n\r\nImprovements tested locally:\r\n\r\n####
`many_fields` dataset (no timestamp)\r\n\r\n- `main`: `~22s` and 10 data
grid rerenders until many_fields data set\r\nloaded. The transform
config dropdown are hardly usable and super slow,\r\neach edit causes 3
data grid rerenders.\r\n- This PR: `~4.5s` and 7 data grid rerenders
until many_fields data set\r\nloaded. The transform config dropdowns are
a bit slow but usable!\r\n\r\n#### `kibana_sample_data_logs` dataset
(whole dataset in the past to\r\ntest rerenders on load without
data)\r\n\r\n- `main`: 5 rerenders.\r\n- This PR: 3 rerenders\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [x] This was checked for breaking
API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"869ceec5ca8a1156d077bb2a888a91ef73e30511"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Walter Rafelsberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels Feature:Transforms ML transforms :ml release_note:skip Skip the PR/issue when compiling release notes v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants