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

Conversation

schveiguy
Copy link
Member

Library code may conservatively call assumeSafeAppend on an array without knowing if the block will be appended to. However, if the block is not cached in the block info cache, it would not add it to the cache. This meant that code that continually calls assumeSafeAppend is constantly looking up the block info from the heap. This fixes that situation.

@@ -637,7 +637,8 @@ extern(C) void _d_arrayshrinkfit(const TypeInfo ti, void[] arr) /+nothrow+/
auto tinext = unqualify(ti.next);
auto size = tinext.tsize; // array element size
auto cursize = arr.length * size;
auto bic = __getBlkInfo(arr.ptr);
auto isshared = typeid(ti) is typeid(TypeInfo_Shared);
auto bic = !isshared ? __getBlkInfo(arr.ptr) : null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auto bic = isshared ? null : __getBlkInfo(arr.ptr);    

@schveiguy
Copy link
Member Author

Updated with suggestions.

@MartinNowak
Copy link
Member

Cache on read, makes sense.

@MartinNowak
Copy link
Member

Auto-merge toggled on

MartinNowak added a commit that referenced this pull request Mar 24, 2015
When calling assumeSafeAppend, cache the block info for subsequent calls
@MartinNowak MartinNowak merged commit 95737c0 into dlang:master Mar 24, 2015
@schveiguy
Copy link
Member Author

I actually considered last night, why put the cache only for array appending, why not just have a general GC.query cache? I doubt there are many occasions to use GC.query, and this would kind of move it into the right place (right now, there is this dance during collection where the GC tells the runtime about destroyed blocks so it can update the block info cache). Thoughts @MartinNowak?

@schveiguy schveiguy deleted the assumesafeappendbic branch March 24, 2015 13:20
@schveiguy
Copy link
Member Author

Hm... but the cache depends on knowing that the block is unshared. Maybe that wouldn't work in the general case.

@MartinNowak
Copy link
Member

We will add thread local caches for the GC soon, and one part of the will be to cache the info, making a separate cache unnecessary.
But yes, it would be a good first step to move the cache to GC.query.

@schveiguy
Copy link
Member Author

Yeah, a GC that knows which blocks are shared or not seems like a prerequisite. Looking forward to that!

@MartinNowak
Copy link
Member

That's just an optimization to avoid synchronization, but the GC has to deal with that anyhow.
As I already discussed with you we can't afford to synchronize attributes, so appending to an array from two threads might perform superfluous reallocs.

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.

3 participants