[Backport 7.x] Use control comm target in LabManager

[martinRenou]

Backport of #3335

[martinRenou]

Making it draft, I need to apply changes from #3335

[jasongrout]

@martinRenou said he plans to work on this this week for an upcoming 7.x minor release.

[martinRenou]

It's updated :)

[tkrabel-db]

@jasongrout @martinRenou I hope you don't mind if a stranger (at least for @martinRenou) asks questions in this thread :) I see that you have the notion of _loadFromKernel and _loadFromKernelSlow. I'm wondering: how slow is the old comm_info_request <> state update logic compared to the new one? Any numbers?

[martinRenou]

Hey @tkrabel-db :) We did some speed tests on the Voila implementation (which is exactly the one added in this PR), see voila-dashboards/voila#766 (comment) and voila-dashboards/voila#766 (comment).

[jasongrout-db]

@martinRenou - do you mind if we rename _loadFromKernelSlow to something that indicates the actual difference rather than the symptom? Perhaps _loadFromKernelSeparate or _loadFromKernelModels or something? I suppose we can rename _loadFromKernel to _loadFromKernelControlComm or _loadFromKernelWidgetManager or something to make the distinction even clearer.

As we've talked about before, it may be that _loadFromKernel ends up with a websocket message that is too big and gets dropped, in which case _loadFromKernelSlow is actually faster :).

[jasongrout]

@martinRenou - I refined the loadFromKernel function to handle a few subtle asynchronous issues. For example, when a bunch of models are being created, it is important for the model registration to happen synchronously before any of the models are actually created, since one model might depend on another model in the same dump of data.

I also noticed that it seemed we were using the same buffers slice, so I made sure to group buffers along with buffer paths.

[martinRenou]

Thanks!

[jasongrout]

@martinRenou looked at these last changes: https://gitter.im/jupyter-widgets/Lobby?at=62062b2803f27047822cde0c

👍 I was just looking into it. This makes sense, thanks!
we should forward port those changes to main

[ibdafna]

@meeseeksdev please backport to master

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout master
git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -m1 c419bfecf50d999fa56e9366cc7c22fa53660ff3

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #3337: [Backport 7.x] Use control comm target in LabManager'

4. Push to a named branch:

git push YOURFORK master:auto-backport-of-pr-3337-on-master

5. Create a PR against branch master, I would have named this PR:

"Backport PR #3337 on branch master ([Backport 7.x] Use control comm target in LabManager)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.