fix: Fix slow powerbi report #650
Conversation
|
Howdy, wanderer🌵🤠🐴, Seems you've sauntered into our GitHub saloon with a pull request, but it appears you've forgotten to tie your horse to the hitching post. Now, in this town, we don't take kindly to stray requests, and the GitHub corral is no place for them. I reckon you best mosey on over and link that pull request to an issue, lest you want the winds of open source trouble blowin' your way. I've got my eye on you, stranger, and a stern warning echoes through these digital canyons. Now, for those who might be new to these parts or sufferin' from a bout of forgetfulness, fear not. I've rustled up a guide that's as handy as a snake in a boot🐍🥾. Take a peek at this guide, and it'll show you the way to tether that pull request like a seasoned rancher🤠. Don't let the sun set on your unlinked pull request, and remember, in these GitHub lands, the code speaks louder than six-shooters. Sincerely, |
| ); | ||
|
|
||
| return filters.filter((f) => !f.type.startsWith(HIDDEN_FILTER_PREFIX)); | ||
| const processSlicer = async () => { |
There was a problem hiding this comment.
Would it not be better to divide the list of slicers into batches, then process the batches?
I think it is much more readable and maintainable to do it in a more functional way :)
There was a problem hiding this comment.
The slicers do not load at the same speed, this way all the workers can always be doing something
| const slicers = visuals.filter((visual) => visual.type == 'slicer'); | ||
| const filters = await Promise.all( | ||
| slicers.map(async (slicer) => { | ||
| const slicerData = await getSlicerDataAsync(slicer); |
There was a problem hiding this comment.
I dont understand the performance benefit of this diff. Does an array of Promises resolve sequentially. That doesnt make any sense. Are you sure the issue is not over awaiting?. How did you benchmark this? Can you try removing the await from inside the .map?
There was a problem hiding this comment.
Previously it sendt all the requests at once which met the max number of allowed concurrent requests in the browser. This implementation caps it at 3 requests running at once. Removing the await would not change anything because the issue is caused by the number or requests.
I do not have any specific numbers but it cuts the loading time down to a few seconds
There was a problem hiding this comment.
So its a browser limitation for how many concurrent api calls towards a single domain?
Seems like it would be better solved by just deferring filter loading until the reportloaded event?
There was a problem hiding this comment.
Yeah there is a max limit pr domain i think the max is 6. Waiting for the report to load is also an option, but that would still cause issues with any other actions the user wants to take while the slicers are loading. The limit doesn't slow down the filter loading as much as you would expect and I think it is the best solution
There was a problem hiding this comment.
That would still cause issues with any other actions the user wants to take while the slicers are loading? I didnt get this?
There was a problem hiding this comment.
I assume that the user wont be able to send any other requests to the domain while they are loading. I am not sure how big of an affect that would have but i think it is better to be on the safe side
|
Howdy, wanderer🌵🤠🐴, Seems you've sauntered into our GitHub saloon with a pull request, but it appears you've forgotten to tie your horse to the hitching post. Now, in this town, we don't take kindly to stray requests, and the GitHub corral is no place for them. I reckon you best mosey on over and link that pull request to an issue, lest you want the winds of open source trouble blowin' your way. I've got my eye on you, stranger, and a stern warning echoes through these digital canyons. Now, for those who might be new to these parts or sufferin' from a bout of forgetfulness, fear not. I've rustled up a guide that's as handy as a snake in a boot🐍🥾. Take a peek at this guide, and it'll show you the way to tether that pull request like a seasoned rancher🤠. Don't let the sun set on your unlinked pull request, and remember, in these GitHub lands, the code speaks louder than six-shooters. Sincerely, |
Short summary
Limits the number of concurrent requests to slicers to prevent filters from blocking the report from loading
PR Checklist
Tip
You can test your changes on the test-application. You can find it under apps\test-app\src\index.tsx
Caution
⛔ I understand by merging my PR, the changed packages will be published to NPM immediately ⛔