microsoft-graph-toolkit icon indicating copy to clipboard operation
microsoft-graph-toolkit copied to clipboard

Batch requests between separate mgt-person components

Open RobPethick opened this issue 5 years ago • 17 comments

Proposal: Improve performance by batching requests between separate mgt-person components

Description

When I put 50 mgt-person cards on the page I can see that the requests for the user and their photo are batched up (so I see 50 rather than 100 requests) but it would be great if the requests could be batched between the different components.

Rationale

This seems like a fairly common use-case and 50 API requests does put me off using this a fair bit.

Preferred Solution

Ideally this would just work out the box though I understand if this is not technically possible I don't know how possible it is for the components to communicate.

RobPethick avatar Sep 21 '20 15:09 RobPethick

Hello RobPethick, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

ghost avatar Sep 21 '20 15:09 ghost

@RobPethick, I agree that batching the Graph calls across components is a great aspiration. Today, we have one off calls to the Graph in each component. We do cluster some calls, but most components act independently. There would need to be some sort of mechanism created to manage the calls in and out of the toolkit for all components.

Back in my Silverlight days I used something aptly named the "WebRequestMaker", to manage all web requests across the app. Perhaps we could introduce a similar concept to intercept outgoing Graph calls and find opportunities to batch them.

This will definitely require some thought and prototyping. Any help is much appreciated ;)

shweaver-MSFT avatar Sep 22 '20 21:09 shweaver-MSFT

Hello,

Any progress regarding this point ?

I guess this isn't a simple issue to resovle but it forbids the usage of the toolkit for any usage that display more than 5 five people at a time.

thanks

stevebeauge avatar May 31 '21 14:05 stevebeauge

I wonder if this could be solved using Graph SDK middleware that could be the central point to manage requests. Recently I used this approach to see how we could avoid issuing multiple requests to the same endpoint, which is slightly related to this issue. See https://blog.mastykarz.nl/avoid-duplicate-requests-microsoft-graph-javascript-sdk/ for more info.

waldekmastykarz avatar Jul 23 '21 12:07 waldekmastykarz

@sebastienlevert I recall you mentioning a project that did something to automatically roll requests into batches Can you share that again please?

gavinbarron avatar Jul 20 '23 00:07 gavinbarron

So @mgwojciech provided this solution for SPFx-based solution, but I think we could extend this easily to MGT. Marcin, would you be willing to help us build something like that?

sebastienlevert avatar Jul 20 '23 14:07 sebastienlevert

I think with middleware @waldekmastykarz mentioned it should not be a problem. I don't think I'll find some time within next 10 days, but if it can wait till August I'll be more than happy to help.

mgwojciech avatar Jul 20 '23 15:07 mgwojciech

It's vacations and we're not expecting you to spend anytime here at all ;) We can connect back in August and we'll definitely be happy to collaborate and work together on design / implementation if you want to help us!

sebastienlevert avatar Jul 20 '23 17:07 sebastienlevert

Sounds like a plan 🙂

mgwojciech avatar Jul 20 '23 17:07 mgwojciech

Here is a sample: https://github.com/mgwojciech/unit-test-samples/tree/main/mgt-batching run npm install Add clientID to .env file npm start You can add more compoments if You want to test it a little bit further. In this sample I just added login, Person, Agenda, Files and Todo, and it looks like all requests are batched. I implemented batching only for get requests as Graph Batch in SDK supports only get (or at least it looks that way).

All the magic happens in https://github.com/mgwojciech/unit-test-samples/blob/main/mgt-batching/src/GraphAutoBatch.ts If You would like some more details on the implementation - don't hesitate to ask

mgwojciech avatar Aug 11 '23 12:08 mgwojciech

I know it's not the best quality code in the world, but I wanted to make it easy to copy-paste if needed.

mgwojciech avatar Aug 11 '23 12:08 mgwojciech

Pinging @gavinbarron on this issue. I think this is a pretty interesting approach. I'm wondering how this would fit in a complex application (large apps with hundreds of calls that would need to be batched across multiple batches, for instance... Thinking about a list of 100+ records with a person component).

I'm also wondering if there are "real" performance benefits or we would just hide a lot of the complexity in a batch and debugging would be harder... But it really sounds promising if the value delivered is the right now.

Definitely I would probably make this a prop on the IProvider interface that we could set to enable this feature (or not) and it would be turned off by default.

sebastienlevert avatar Aug 15 '23 23:08 sebastienlevert

I have this feeling there is a way to do this with middleware. If that would be the case it would be more elegant and easier to integrate in mgt itself. I will keep this in my background thread this week and try to push it next week (if I come up with something). When it comes to multiple records - I did some testing and it looks like there is something in the component that slows it down. Like it waits for data for all components of one type to render all at the same time. I'll investigate further :)

mgwojciech avatar Aug 16 '23 06:08 mgwojciech

Thanks @mgwojciech! This is really appreciated! Happy to brainstorm in the next weeks on this approach!

sebastienlevert avatar Aug 16 '23 20:08 sebastienlevert

it's an interesting approach, there's a bit of a downside in that we might lose component level telemetry, it just depends on how the request headers for each underlying request in the batch are unpacked and processed by the telemetry pipeline.

I think that the middleware idea is really interesting and potentially more flexible, as then auto batching could be leveraged by any customer using the JavaScript SDK.

Awesome stuff @mgwojciech!

gavinbarron avatar Aug 17 '23 17:08 gavinbarron

Any updates on this? We just encountered the same problem.

carlo-quinonez avatar Jun 10 '24 16:06 carlo-quinonez

Hi @carlo-quinonez

If I remember correctly, the issue is that mgt-persona batches user, presence and photo per user and it's impossible to batch a batch.

I ended up with implementing my own batch library and persona card. If You feel like it, find the library here: https://www.npmjs.com/package/mgwdev-m365-helpers

Here You can find a sample implementation using Fluent9 https://github.com/mgwojciech/MGWDev.SPEmbedded/blob/master/client-app/src/components/GraphPersona.tsx This particular sample is using react global context, but You can easily replace it with passing graph client as component property if that makes more sense in Your case.

In general, I don't think it's feasible to actually fix that in mgt, as I believe it would force quite a refactoring.

As a bonus, take a look at beta profile endpoint, can be quite useful for extended properties: https://learn.microsoft.com/en-us/graph/api/profile-get?view=graph-rest-beta&tabs=http

mgwojciech avatar Jun 11 '24 06:06 mgwojciech