vlang / v

Simple, fast, safe, compiled language for developing maintainable software. Compiles itself in <1s with zero library dependencies. Supports automatic C => V translation. https://vlang.io
MIT License
35.79k stars 2.16k forks source link

All Array modifiers that Add or Remove elements need to follow the rules for slices and avoid unnecessary memcp when possible. #11223

Open lancethepants opened 3 years ago

lancethepants commented 3 years ago

V version: V 0.2.2 405ed58

OS: All

What did you do? Try to add and remove elements in an array to see if it obeys the rules for slices while also avoiding unnecessary memcp when possible.

https://github.com/vlang/v/pull/11155 https://github.com/vlang/v/issues/11150

What did you expect to see? All array modifiers are aware if an array is in parent/child(slice), and don't do any unnecessary memcp

What did you see instead? Some are slice aware, some aren't. None take into consideration both scenarios. 1 path for slices (using memcp) , and 1 path for non-slices (using memmov). Really I think the non-slice scenario (memmov) is a much broader use case, so I think optimizing for these scenarios is pivotal.


https://github.com/vlang/v/pull/11155#issuecomment-898623409

Any time an array is modified by adding or removing an element we need to check if it is a parent or child (ie. shares memory space) with another array (or slice). This check currently does not happen.

If the array is a slice (child), and we add or remove an element, then the child becomes independent from the parent by creating a copy.

The V documentation only mentions becoming independent when adding to a slice (child), or a parent array when it also exceeds its cap. However, this comment says it should also be extended to parent and child whenever you remove an element. https://github.com/vlang/v/pull/11155#issuecomment-897815709

Currently only some array modifier take into account the possibility of an array parent/child scenario. <<, delete(), delete_many() are the ones I'm aware of. However, in their current form they unconditionally create a copy just in case there might be a parent/child array relationship. This is the check that needs to take place mentioned earlier. This copy adds a lot of overhead and can be expensive when many elements are being added or removed.

From the Docs this is all the array modifiers I could see that add or remove from arrays. They all need to follow the rules for slices and also have non-slice scenario optimizations. None so far fulfill both requirements.

repeat()
insert()
prepend()
trim()
clear()
delete()
delete_many()
pop()
delete_last()
yuyi98 commented 3 years ago

Sorry, I don't agree with this rule. It's too complicated and will cause more problems. If you want to use slice of the parent array independently, you can use clone. If you want to manipulate slice values efficiently, you should make sure that parent doesn't realloc.

lancethepants commented 3 years ago

Honestly my goal is to be able to manipulate arrays in V as efficiently as in golang. Some functions are efficient, some aren't. Some take slices into account where others do not. Sometimes V matches go in speed. Other times go runs circles around V. Right now there is a big inconsistency with how V handles arrays.

Personally I haven't made much use of V slices. I want to see V arrays be efficient, but I'm just recognizing that slices are currently part of that equation. I don't have a dog in the fight when it comes to slices in particular, I just want to notify leadership and decision makers that arrays functions aren't always the most efficient and the handling of slices is the current roadblock to this. Unconditionally copying the contents of an array on grow or shrink to another location because a slice might be indexing your array (but no check takes place) is inefficient.

ntrel commented 2 years ago

insert() - in place, has to move elements prepend() - calls insert trim() - doesn't copy anything clear() - doesn't copy anything delete() - calls delete_many delete_many() - usually duplicates the array unnecessarily pop() - doesn't copy anything (recently no longer calls memdup just for one element unnecessarily) delete_last - doesn't copy anything

So the only ones I think are problematic are delete and delete_many. Perhaps update the issue title?