Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

Conversation

denis-sh
Copy link
Contributor

An attempt to create a function to use in std.array.appender and anticipated (let's correct: needed in most cases IMO) "real appender" from Issue 11138.

I'm not good in D internals but what happens now in std.array.appender looks like a total mess as it attempts to create D array but doesn't respect any hidden info at all.

Accidentally it is requested in Issue 9313 - Provide dynamic array-dedicated "extend" function.

@denis-sh
Copy link
Contributor Author

I'll also be glad to see alternative solutions and causes current std.array.appender is done as it is. If we have such code even in Phobos let alone regular users there are problems in language design and terrible pitfalls. Or am I just misunderstanding something?

@MartinNowak
Copy link
Member

It will take some time for me to wrap my head around this so it won't happen too soon.

@monarchdodra
Copy link
Contributor

There should be an associated public core.object.extend function no?

@denis-sh
Copy link
Contributor Author

There should be an associated public core.object.extend function no?

Yes, but probably in another pull as I don't want discussion about public API change to delay merging of this.

@quickfur
Copy link
Member

ping

@quickfur
Copy link
Member

Merge conflict. Please rebase.

@DmitryOlshansky
Copy link
Member

This looks useful - any chance we can restart it?

@schveiguy
Copy link
Member

I'm trying to understand the difference between requested capacity and desired capacity. The documentation is not really helpful.

@denis-sh
Copy link
Contributor Author

I'm trying to understand the difference between requested capacity and desired capacity. The documentation is not really helpful.

E.g. when we are building an array T[] and want to put one more element in already full buffer we require at lest T.sizeof bytes but generally allocate more storage e.g. currentCount * 2 * T.sizeof bytes to reduce allocations count.

@schveiguy
Copy link
Member

I think part of my confusion is I assumed reqSize meant requested size, but now I think it means required size.

So let me throw this strawman definition out there:

reqSize -> Size you absolutely need the array to grow to.
desSize -> Preferred size to grow to if reallocating is necessary

Algorithm:

  1. If the current capacity exceeds reqSize, nothing is done (current capacity is returned)
  2. If the current block can be extended to a size between reqSize and desSize, the largest possible extension in that range is selected, and the new capacity is returned (not sure if this is the case exactly, I have to check gc_extend).
  3. Otherwise, the GC attempts to allocate a block the size of desSize. If it succeeds, then the new capacity is returned.
  4. If reqSize is not desSize, reqSize is also tried if desSize fails. If it succeeds, then the new capacity is returned.
  5. If the array cannot be extended, out of memory error is raised.

An interesting optimization would be to allocate a block between reqSize and desSize if extension isn't possible. But maybe that's not such a good thing, if you are running low on memory, better not consume it all.

If this is the case, can you put something similar into the docs? Also document better the "ability to discard current array" -- just say that it calls postblit on original array if true :)

I like the idea, BTW. Wish I would have done arraySetCapacity this way to begin with :)

@andralex
Copy link
Member

ping - rebase and mind failures?

@MartinNowak
Copy link
Member

It will take some time for me to wrap my head around this so it won't happen too soon.

Last time I thought about this (~1 year ago) I couldn't come up with a single use-case where I require a certain amount of memory but actually want more in order to do something more efficient.
We already provide .capacity to query additional memory, so I don't think this is particularly useful.

@denis-sh
Copy link
Contributor Author

Last time I thought about this (~1 year ago) I couldn't come up with a single use-case where I require a certain amount of memory but actually want more in order to do something more efficient.

Interesting. This pull is just a practical requirement for array builder.

@MartinNowak
Copy link
Member

Interesting. This pull is just a practical requirement for array builder.

You mean appender? How is that? Appender already knows the how much memory was actually allocated and can reuse that until it needs to realloc.

@denis-sh
Copy link
Contributor Author

You mean appender?

Appender and its future replacement Builder I planed to add that time.

How is that? Appender already knows the how much memory was actually allocated and can reuse that until it needs to realloc.

When we are out of capacity, we need to grow. And to grow we need a function which accepts how much memory we do require and how much memory we just want to be allocated to decrease future allocations count.

As we currently lack such function in druntime this pull was created.

@MartinNowak
Copy link
Member

And to grow we need a function which accepts how much memory we do require and how much memory we just want to be allocated to decrease future allocations count.

There are two schemes that already work equally well.
Allocate as much as you require and use the excess you got or
allocate as much as you want and fail if that doesn't work.
The additional sophistication is really unnecessary and creates new problems because now the grow strategy needs to be tuned to the allocation size classes of the GC.

@DmitryOlshansky
Copy link
Member

The additional sophistication is really unnecessary and creates new problems because now the grow strategy needs to be tuned to the allocation size classes of the GC.

Looks like it's not appreciated and was there for 2 years.Closing

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants