Fix for #719 #732
Fix for #719 #732
Conversation
Several changes in the way the Tables enumerate its fields. - No longer flatten the source to display the table view. - Use fromSources filter to dig into source hash structure to find fields when displaying the view - Trigger actual field loading on fields list opening - Use current search result to update the field list when the list is already visible in view at search time - Cache micropanel flattened data - Table view flatten data when drawing
|
Let me know if this is better :) At least the micro panel and table view seem to display correctly now... |
| @@ -81,7 +81,7 @@ | |||
| <th>Action</th> | |||
| <th>Value</th> | |||
| </thead> | |||
| <tr ng-repeat="(key,value) in event.kibana._source track by $index" ng-class-odd="'odd'"> | |||
| <tr ng-repeat="(key,value) in flatten(event.kibana._source) track by $index" ng-class-odd="'odd'"> | |||
There was a problem hiding this comment.
This is considered a bad practice in angular. flatten will need to be evaluated on every digest. As the user opens more rows this will get expensive quickly.
There was a problem hiding this comment.
Indeed, but is this worse than flattening all rows at once when the table renders initially ?
Maybe the code could refactored much more so that it flattens all rows when needed, once, when opening the fields list, showing a row as a table, or micro-analysing a facet. And still, we don't need to flatten all rows when just showing a single row as a table
Right now, my knowledge of angular is pretty much experimental, hence the bad practice result :/ What's your opinion ?
|
Thanks for working on this!! FYI, I checked out your FIX_FIELD_LIST_SLOWING branch from your github: https://github.com/fotonauts/kibana/tree/FIX_FIELD_LIST_SLOWING However I did not see much improvement. Do I need to do anything special? (I might be preempting here, so sorry if you're not done.) |
|
@bryanyork Hi, the fix is supposed to let the table display faster provided the field list is collapsed. If the field list is already opened, it will not change. I hope you can see some speed improvement when the list is collapsed at page load. My data only contains 101 fields and my test case might not be exactly yours :-/ To be sure this is actually my branch running, you should see the "loading" indicator spin when you open the field list. Let me know if this makes some difference... |
|
Hi again. So yes I can confirm that I see the loading indicator spin when I open the field list. My issue is not the page load speed. (If you're solving another issue please let me know.) My issue is the browser is super slow after everything is loaded and I try to input text into my search box. CPU usage is very high. This is after everything is loaded. (I can get the CPU usage to go down if I minimize the Table panel.) Thanks again! |
|
This is not what I had understood and found on on my own Kibana instance. I'll have a look at that part too. After all, this PR is not yet ready for prime time :D |
|
Hey @bryanyork , I've started looking at the code and something is curious with the way Angular watches all the changes. On my setup, an event fired by the keypress in the search field takes 298ms and checks 20786 watchers in the page. If you want to apply the patch at https://gist.github.com/octplane/7751877 , I'm sure you'll find some interesting values, especially the time taken to call all the watchers. Of course, as soon as you remove the table, it goes down to 34ms for 3982 watchers which is much better. I'll keep on staring at the code for a while 😴 |
|
Hey @octplane thanks so much for your hard work. I enabled your diff and found that keypresses were at 418ms with around 20,000 watchers. @rashidkpc I tried the latest pulls, and performance seems to be a tad better, but still not ideal for me. Also, getting a lot of errors now in the javascript console: TypeError: Cannot call method 'split' of null Let me know if I can help in any way. |
|
@bryanyork Did you clear your browser cache after updating? Not just a hard refresh, but a proper clear? |
|
@rashidkpc Ah you're right, that did the trick. I consider this bug solved!! Thanks so much to you both! Loving Kibana. |
|
@rashidkpc feel free to close the PR :) |
* master: Moved doc task to the end of the default task chain removed y_as_bytes, replaced with y_format Remove console.log Closes elastic#538. Closes elastic#722 Potential fix for elastic#621 More small table performance improvements Improve performance in the table. Re elastic#719 and elastic#732 Remove idQueue from filter and query services in dashboards Eliminate idQueue property from filterSrv and querySrv. Replace with binary search for smallest id. Closes elastic#730. Closes elastic#739 Skeleton API docs Doc updates Added scratchy tasks and config.js docs Bytes should not be default Fixing unsafe html binding fixed numeric terms in topN query do not auto-enable saved filters
* [WIP][Design] Cleanup canvas part 1 (elastic#697) Part 1 of dave messing around with the design of canvas. * Converted bootstrap button groups to EUI button groups in text_style_picker (elastic#713) * Moved elements and assets to workpad_header (elastic#714) * Adds tabs to sidebar component (elastic#718) * Changed sidebar to tabbed layout * Removed unecessary done props from datasource_component and expression * Removed unused props in toolbar * [Design] Part 2 of design cleanup (elastic#715) * hide nonfocus pages * button group margins * workpad panel styled * header rework * fix toolbar lint * update to newest EUI, restyle sidebar tabs, some data source cleanup * move title to footer * Add some tooltips * Fixed custom interval input in refresh controls (elastic#722) * Fixes Editor Toggle (elastic#721) * Removed unused selectedPage prop from workpad_header/index.js * Fixed editor toggle * Design part 3: page view, data sources, lots of cleanup (elastic#731) Mostly changes around the page selector and data sources. Also fixes some of the hover issues for canvas elements. * fix css import error * [Design] Part 4 of design updates to canvas (elastic#732) [Design] Part 4: selector rewrite * Move expression editor back to bottom (elastic#733) * Removed unused prop form sidebar_component * Moved expression editor to toolbar * [Design] Part 5: Cleanup and bugfixing before review (elastic#734) * style up expression editor, fix resize handles * hide controls when editing is hidden * remove icons from element dropdown * fix canvas grid * Adds Modals (elastic#739) * Changed datasource_preview to a modal * Moved workpad_loader to modal * address feedback (elastic#740) * asset manager mostly styled (elastic#741) * [Image function] support src (image url) in addition to dataurl (elastic#632) * [Image function] support src (image url) in addition to dataurl * unit test for httpurl * fix other elements that use image * clean up the diff * more clean up diff * Handle AJAX errors by showing a notification (elastic#717) * timeout property for axios instance * remove TODOS * notifier class * TODO * second notification with the context * redirect to home if workpad could not be fetched * less noisy error checking * Fix ES Docs indices select options (elastic#743) * Catch more AJAX errors, show error in notification banner (elastic#736) * catch more errors * if es_fields fails, still render something * generate default data in ajax fetches in case of failure * feedback changes * remove defaulting that catch makes unnecessary * Remove run API (elastic#742) It's not used anywhere and is no longer necessary. We can always recreate it if the need arises. * [Design] Part 4 of design updates to canvas (elastic#732) [Design] Part 4: selector rewrite * Changed icon to 'warning' in simple_failure * Fixed canvas loading * Fixed paginate controls file naming * Changed color dot size * Added missing aria-labels * Fixed warnings * Fixed expression warnings * Fixed popovers * Fixed merge conflicts for image_upload * Changed placeholder text in custom interval input * Styled debug render function * Removed bootstrap from expression and show_debugging * Changed buttons in expression editor * Changed asset manager to modal * Added 'canvas__element' class for BWC for custom CSS in old workpads * Fixed fullscreen interactions and positioning * Fixed overflow issue in asset manager * Removed bootstrap in datasource component * Changed text input to text area in timelion datasource * Fixed sort field in esdocs * Updated style of AssetManager * Changed all inputs to compressed size * Added max width for datasource_preview modal * Fixed cursor when hovering over element in fullscreen mode * Cleaned up tooltips * Rearranged buttons in workpad_header * Updated style to update_modal * Forgot to remove debug line * Cleaned up arg_add_popover * Removed codeblock in update_modal * Added 'dataurl=null' to image element initial expression * Added null args to repeatImage and revealImage * Fixed input validation in expression * Bumped eui to v1.1.0 * fix: make refresh controls clearer - Change disabled state to 'refresh this page manually' - Don't show disable control unless auto-refresh is enabled - Move disabled control under the refresh interval text closes elastic#759 * fix: close refresh popover on selection * fix: replace interval placeholder with help text Closes elastic#757 * fix: replace EuiFormLabel with aria-labelledby closes elastic#767 * fix: restore send to top/bottom controls * fix: more usable page controls - pull controls out of popover - move PageControls component out of the preview - fix link use, so controls actually work - fix page layout for tall pages * fix: use correct esdocs query value * fix: better esdocs form labels * chore: stricter prop type checking and remove an unused prop check * fix: style the unknown args datasource fallback * fix: show correct sort field in esdocs * fix: page control icon colors * Fixed workpad_loader modal issue * Cleaned up home page styles * Fixed class name on confirm modal. Removed unused style in main.scss * fix: flatten the workpad styles easier to override them, which is handy for fullsceen and other cases * fix: better fullscreen overrides * chore: remove unused allowFullScreen props * fix: workpad size in fullscreen mode * chore: convert Positionable to a class component * fix: map font object to flot spec * fix: use flot font spec for flot output * test: add font spec test, fix plot & axisconfig tests * Fixed input refs in datacolumn * Removed unnecessary styles * chore: remove debugging console logs * Fixed prop type error in expression form * Fixed expression form bouncing from error messages * fix: page controls visible with scroll bar * fix: delete element click handler * chore: bump EUI to 3.0.0 this is the version that will be landing in Kibana * fix: match ContextMenu class to css rule * chore: remove unused code * chore: tiny update modal code refactor * Replaced & with prefixes in asset_manager and suggestion SCSS files * Replaced EUI link/icon with EuiButtonIcon * Changed label for index pattern in esdocs form * fix: add aria-label to EuiButtonIcon * fix: restore highlight on hover and remove the now unused ElementControls component * chore: update classNames use the new class naming convention * chore: remove unused components * Disabled selection and dragging for img in revealImage * Add toolbar tray close button (elastic#842) * Fixed range and percentage arg types (elastic#843) * Change to preview images * Chore: naming convention * Fix EUI Tooltip component content props (elastic#850) * Fix a small typo in a button icon and tooltip (elastic#851)
Several changes in the way the Tables enumerate its fields.
fields when displaying the view