Siccity / xNode

Unity Node Editor: Lets you view and edit node graphs inside Unity
MIT License
3.36k stars 593 forks source link

NodePort generates garbage on GetConnections #148

Open lucasmontec opened 5 years ago

lucasmontec commented 5 years ago

The method GetConnections() in line 210 of the class NodePort.cs, generates a list (line 211) for every call. This list could instead be in the class and be cleared every time the method is called, just updating references.

Siccity commented 5 years ago

I think silently altering a list after returning it could lead to some frustration if a user decides, for some reason, to store a reference to the list. How about a void GetConnectionsNonAlloc(List<NodePort>)?

Alternatively you can use ConnectionCount and GetConnection(int).

lucasmontec commented 5 years ago

GetConnectionsNonAlloc(List)?

That would be perfect. Also, after trying to work with xNode I found really hard to extend it. All classes are very tight and allow for little modification. You should add more protected methods instead of private, or at least provide interfaces for us to implement our own versions of the same classes. I could do a PR for that if you want.

Siccity commented 5 years ago

Which methods do you mean in particular? I try to make it as open as possible, but I also sometimes mark methods private so internal stuff doesn't bloat the API

Siccity commented 5 years ago

As for interfaces, I'm already working on it for V2 at https://github.com/Siccity/xNode/tree/dev It's a huge undertaking with many considerations to be taken.

lucasmontec commented 5 years ago

Which methods do you mean in particular? I try to make it as open as possible, but I also sometimes mark methods private so internal stuff doesn't bloat the API

The main methods I was working with were Node methods. I wanted to override how they work to improve performance and reduce garbage to 0. We did this a while ago, me and a friend. We removed 100% of all allocations that xNode was doing. Since we use it in runtime and call some methods every frame, we needed to remove any allocations. But recently I wanted to update xNode to get all of the bug fixes and other things being developed, so I had to revert every change we made.

We changed mostly the usage of foreach, the creation of lists and other temporary variables. I don't remember exactly which methods, but having interfaces will solve this problem.

Thanks for xNode! This tool is indeed amazing. I'll wait for V2 then.

lucasmontec commented 5 years ago

Have a coffee. That will replenish your energy ;)

Siccity commented 5 years ago

So here's an update. I tried making a NonAlloc method, but adding the ports to an existing list also creates garbage. I discovered ReadOnlyCollection and made a branch which is still very early in development. Performance wise it's much faster though.

https://github.com/Siccity/xNode/tree/dev-connections-performance