-
-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Facilitate implementing core data structure such as linked list via modules or plugins #1522
Comments
Note, an array can be just as fast for this goal if capacity is properly implemented (and I would use one when I need a stack). |
I've done some profiling with push back/front on both With 10000 elements: So yeah Also see comment made by @zwparchman goostengine/goost#12 (comment) and useful article on vector/list performance. |
Made out of curiosity. See godotengine/godot-proposals#1522.
@Xrayez if you want to see my point with array, you should not recreate it from scratch every time (and test in release_debug). The point of capacity is to act as re-usable, contiguous memory pool so that the next |
I won't go deep into profiling because that's not my cup of tea (at the moment!), but if I understand you correctly, here are results (still recreating the types, but I haven't seen much difference): 20000 elements,
|
I have renamed the proposal to something which reflects my initial idea more accurately, I do not necessarily propose implementing an equivalent data structure in core, but there are quite a lot of issues I've stumbled upon while implementing this via C++ module which deserved a proposal, because most reported issues can only be resolved on the core level. Fixing most core issues can resolve this proposal (some of which I can do myself, and some already did), so I hope those issues are objective enough which make this proposal actionable. A lot of those issues are agnostic to the proposal, and could help implement other data structures in C++ more easily, the most prominent and important out of all is probably godotengine/godot#42060, because it took me quite some time to figure this out on my own, and could help a lot of use cases, opening the door to implementing other more or less efficient data structures (performance is not priority for Godot development out of the box, so makes sense to satisfy performance needs for other parties). |
There's another proposal for graphs #3848 which could greatly benefit from solving bullet-points presented in this proposal. I'd say a graph could also be used as a linked list which could as well remove the need to implement |
If you really, really want to use a linked list, you can simple use a contiguous array and just store the index of the next node with the current item. So each element in your list is something like:
All other graph structures can be implemented in similar fashion. The only thing you pay for with each lookup is the offset from the list pointer. You might even get better performance this way over using a pure linked list because the data is all garuanteed to be in a similar (not as good a vector but still) portion of memory. |
@otoomey Your approach is good for certain circumstances, but a lot of middle inserts or deletions will lead to severe fragmentation and cause the size of the array to grow unnecessarily large. Even a lot of front or end insertions and removals can lead to weirdness with determining whether to loop the array around or extend it. I'm disappointed that this proposal hasn't received more traction lately. The Goost implementation of LinkedList which @Xrayez was working on seems to be completed, though not available for 4.X at least without some porting. Are the core issues enumerated in this proposal still causing problems for that implementation? |
@awardell Hmm I'm not sure if I completely agree. The order in the array does not have to match the order in your linked list. If you have a doubly linked list you can implement it in an array with O(1) insertion and deletion without fragmentation. When inserting, just push the element onto the end of the list and set the indices. When removing, swap the element to remove with the last element in the list and update the indices appropriately. This way you also benefit from improved cache locality. A true linked list would rely on either godot or the OS memory allocator to keep fragmentation low. Therefore, even if I was writing a linked list in C I would implement it this way unless performance is not of concern. |
Describe the project you are working on:
Goost - Godot Engine extension.
Continuing from godotengine/godot#7194.
Describe the problem or limitation you are having in your project:**
Simply put,
List != Array
. Linked lists perform faster when it comes to insertion and deletion operations, havingO(1)
time complexity, not to mention the benefit of passing around list nodes throughout code while still maintaining the order of elements.Note that this proposal also applies to any other core data structure which could be implemented via modules or plugins, but the linked list is one of the most commonly used data structures out there.
Describe the feature / enhancement and how it helps to overcome the problem or limitation:**
For instance, I've previously ported bucket fill algorithms from GDScript to C++ because they were too slow to execute via script, and bucket fill typically requires a stack-based data structure (for which the
List<T>
is used throughout the engine internals). There was certainly a factor of going through GDScript calls for each pixel there as well which contributed to the slowdown, of course.Other data structures could be more efficiently implemented/derived by using the
List
in a particular way, such as stack or queue. Any recursive calls could be more efficiently implemented using a list as a stack without reverting to using the GDScript call stack (which is also limited to 1024 by default). There are ways to use anArray
as a stack as suggested in #1522 (comment), but the usage is advanced and requires more steps to setup and maintain.Binary trees can also be implemented using nested linked lists.
Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:**
As a
Reference
See goostengine/goost#12 where this is already implemented using C++ modules capabilities.
A list is implemented as a
Reference
and the nodes/elements inheritObject
.While working on such an implementation, I've stumbled upon various core issues:
ListNode
in this case) for them to be compatible withVariant
: Allow to cast custom objects forVariant
godot#42060._init()
,new()
etc). This is possible withArray
andDictionary
in contrast.free()
calls for GDScript. ForReference
, this is already enforced by preventingReference.free()
when used in GDScript specifically, but not in core. Preventing or overridingfree()
calls forListNode
s is desired becauseLinkedList
needs to take care of re-linking previous and next nodes upon deletion, otherwise this may lead to a crash attempting to delete previously deleted pointer to a node.print()
for those classes in C++: MakeObject::to_string
virtual godot#42093.for node in list:
to work fast enough without allocating anArray
in GDScript: Custom iterator performance is very poor compared to allocating an array and traversing it godot#42053.List
asList
because of namespace issues in C++ (but possible in GDScript...)LinkedList.front
directly results in an error, which is exposed as a read-only property: GDScript variable chained assignment fails when an intermediate variable is readonly. godot#41319.As a
Variant::LIST
and/orVariant::LIST_NODE
I really don't know what has to be done to make it work, but I guess that would be the same as implementing
Variant::ARRAY
orVariant::DICTIONARY
, so we'll have a third type of container type in Godot:Variant::LIST
. But obviously, it would solve all of the above limitations because it would be treated as an actual core data structure.If this enhancement will not be used often, can it be worked around with a few lines of script?:**
It is perfectly possible to implement such a structure via GDScript, certainly not a few lines of code but possible, see willnationsdev/godot-next@72c0f7f for instance. Yet there are existing issues which prevent implementing such a structure to work reliably via script (not mentioning the above limitations when trying to implement the same data structure in C++):
Is there a reason why this should be core and not an add-on in the asset library?:**
I have to be honest, with the particular
Reference
-based list implementation, there's no reason for this to be in core implemented like that. The engine does haveList
implemented in C++ for internal engine development throughout the codebase but despite this, it's really not trivial to expose the same data structure to scripting. It would be best if the list is implemented as a coreVariant::LIST
type to begin with, I've raised this proposal for discussion purposes, to share my research on this topic, and to indicate what has to be done in order to facilitate the development for modules and plugins.I feel like goostengine/goost#12 could already solve a lot of problems, so feel free to suggest what could be done there as well, because it's not only about Godot Engine development, it's about solving more or less common needs as requested by people. 🙂
In any case, I think there are enough of bullet points to solve in this proposal to make it easier for C++ modules developers to implement such a structure more reliably for everyone.
The text was updated successfully, but these errors were encountered: