Fix T101746: Create copible links by order in multi socket
AbandonedPublic
Actions

Authored by Iliya Katueshenock (Moder) on Oct 12 2022, 9:46 PM.

Details

Reviewers

Hans Goudey (HooglyBoogly)

Maniphest Tasks

T101746: Copy/Pasting changes links order

Summary

Fix T101746: Create copible links by order in multi socket

Implementing reference input in a multisocket requires
order to be preserved. To simplify implementation within a
fix, the copies are simply sorted and created in order. In
the long run, it would be better to improve the multisocket
link connection operations interface. But for now, that's enough.

Diff Detail

Event Timeline

Iliya Katueshenock (Moder) requested review of this revision.Oct 12 2022, 9:46 PM

Iliya Katueshenock (Moder) created this revision.

Iliya Katueshenock (Moder) edited the summary of this revision. (Show Details)Oct 12 2022, 9:49 PM

Merge master
Cleanup

What do you think about P3255? I think it's a more obvious alternative that reuses existing code.

Besides that, I think the performance focused changes in this patch are overly preemptive. It shouldn't be necessary to use a multithreaded sort for sockets connected to a node, that's just a waste of binary size. And the reserve_for function exposes the internals of MultiValueMap which we should avoid because it may change in the future.

This revision now requires changes to proceed.Oct 14 2022, 7:01 PM

The idea of deferred updating for missing links seems reasonable. But it still requires new code to update. And it will have a quadratic size for each node.
Sorting is multi-threaded - yes, I think I was just hoping that somewhere there was a check for too small slices. Yes, it would be better to replace.
But sorting is necessary in any case, since the links are initially in random order in memory. So even a later update would require first making a vector of links (each socket for each link) and sorting them.

Edit: If reserve_for is no in the plans, then yes, I will remove it

The idea of deferred updating for missing links seems reasonable. But it still requires new code to update. And it will have a quadratic size for each node.

This is not quadratic. The runtime is the multi-input socket count multiplied by the number of links connected to each socket.

But sorting is necessary in any case, since the links are initially in random order in memory.

The memory location shouldn't matter at all. And the multi-input socket index is stored in links exactly for that reason-- to avoid relying on the order of the list. No sorting should be necessary.

But sorting is necessary in any case, since the links are initially in random order in memory.

The memory location shouldn't matter at all. And the multi-input socket index is stored in links exactly for that reason-- to avoid relying on the order of the list. No sorting should be necessary.

If there is a missing link, you need to traverse all [miss : end] the links somehow in order to move the end.

And also, from the very beginning you do not know about those links that do not exist. That is, bypassing all the elements following them will also require searching for gaps

My solution was just simple to sort everyone and get new indices. It seems to be easier. Although maybe I don't see any more obvious solution

Iliya Katueshenock (Moder) abandoned this revision.Oct 14 2022, 8:30 PM