-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Array: optimize shift and unshift #10081
Conversation
I believe that in the |
@bcardiff Where do I change that? |
At https://github.com/crystal-lang/crystal/blob/master/etc/lldb/crystal_formatters.py#L14 Unfortunately, the manual specs that can be trigger by https://github.com/crystal-lang/crystal/blob/master/spec/debug/test.sh do not include a spec for Array. |
(or you can change the ivars order 🤷♂️ ) |
@bcardiff Good catch, done! We can't change the ivars order because then the Array will occupy more space. |
FYI, The linux_32 failure seems to be because of a different value of |
Thanks. We can fix that in a separate PR. |
This is exciting! However, how does |
This is a big deal. I would love to see this particular optimization make it into the merge. I am constantly having to refactor code that uses Array's to find different solutions than what comes most naturally. Perhaps I'm just spoiled by Ruby's implementation of Arrays and I need to be less lazy, but old habits die hard and one great reason to use Crystal is precisely because so many Ruby concepts carry over. |
Good question. You can try doing some benchmarks on both data structures and see. My guess is that Deque will be more memory efficient when used exclusively as a deque, compared to Array, but I don't know. For example rotate is more efficient in Deque. I wouldn't remove Deque from the standard library. |
Revival of #8036 and more!
Fixes #5126
Fixes #10079
Closes #5148
How Array#shift used to work
Before this PR, an Array used to consist of a buffer, a size and a capacity. The buffer would have a capacity. As elements are pushed into it the size increases. Once the capacity is reached, the buffer is doubled in size. To do this, we call
realloc
on the buffer.When calling
shift
, because we must remember the exact pointer where the buffer was allocated in order to do arealloc
, the only thing we could do was to call realloc, then move all the elements "to the right":The next time
shift
is called we must do the same thing again, except that, well, we don't need a realloc for a while, but we still need to copy the memory to the right.How Array#shift is optimized
With this PR, Array now also tracks an offset to where the buffer starts. So for example:
This is very efficient because all we need to do is increment a couple of variables: no memory is moved.
Furthermore, if we later
unshift
a value, we can move the buffer to the left and decrement offset.As explained in #8036, adding this
offset
to Array doesn't change its memory consumption, at least not in 64 bits platforms. And in 32 bits platforms it's just 4 more bytes per Array which isn't terrible.How Array#unshift is optimized
This is something that wasn't done in #8036
The idea is that when we
unshift
an element and we have no more space left, we grow the array but leave the pointer to the buffer pointing to the middle of the buffer:In this way if another
unshift
comes we just need to move the buffer to the left. Just after 3 unshifts we'll need to reallocate and move the memory again, and then it will happen after 6 unshifts.Also note how this is similar to
push
but in the other direction: when youpush
and there's no more space, some extra space remains on the left in case morepush
es comes later on. This is exactly the same but in the other direction.Why is this important?
I consider this change extremely important. With this, Array can be used as a list, as a queue, as a dequeue... whatever! Well, previously it could be used like that too, but it wasn't efficient in all cases. So with this, Array becomes a universal, efficient data structure. Just like in Ruby. No more fear of calling
shift
orunshift
and thinking "Ugh, why do I have to pay this penalty?".Benchmarks!
Shift
I know this is allocating an array, but still, look at the times:
Unshift
In both operations, this change leads to speed that's around 2 orders of magnitude faster.
Please
Can we have this in 1.0 pretty please? I don't mind undoing this if we later decide to go with a thread-safe Array. But until then everyone can enjoy a more efficient Array.