Skip to content
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

Move block info caching into the gc package #17067

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions druntime/mak/COPY
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ COPY=\
$(IMPDIR)\core\internal\elf\io.d \
\
$(IMPDIR)\core\internal\gc\bits.d \
$(IMPDIR)\core\internal\gc\blkcache.d \
Copy link
Contributor

Choose a reason for hiding this comment

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

These should be part of the first commit

Copy link
Member Author

Choose a reason for hiding this comment

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

These commits were split to aid in review. The first commit is a straight copy/paste of code. In essence, they are not separable -- you can't build with the first commit only, even if I were to make these changes go into the first commit.

I had a hard time doing the split, don't make me do it again...

Copy link
Contributor

Choose a reason for hiding this comment

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

OK

$(IMPDIR)\core\internal\gc\os.d \
$(IMPDIR)\core\internal\gc\pooltable.d \
$(IMPDIR)\core\internal\gc\proxy.d \
Expand Down
1 change: 1 addition & 0 deletions druntime/mak/DOCS
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ DOCS=\
$(DOCDIR)\etc_linux_memoryerror.html \
\
$(DOCDIR)\core_internal_gc_bits.html \
$(DOCDIR)\core_internal_gc_blkcache.html \
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

$(DOCDIR)\core_internal_gc_os.html \
$(DOCDIR)\core_internal_gc_pooltable.html \
$(DOCDIR)\core_internal_gc_proxy.html \
Expand Down
1 change: 1 addition & 0 deletions druntime/mak/SRCS
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ SRCS=\
src\core\internal\elf\io.d \
\
src\core\internal\gc\bits.d \
src\core\internal\gc\blkcache.d \
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

src\core\internal\gc\os.d \
src\core\internal\gc\pooltable.d \
src\core\internal\gc\proxy.d \
Expand Down
68 changes: 36 additions & 32 deletions druntime/src/core/internal/gc/blkcache.d
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ alias BlkAttr = GC.BlkAttr;
/**
cache for the lookup of the block info
*/
private enum N_CACHE_BLOCKS=8;
private enum N_CACHE_BLOCKS = 8;

// note this is TLS, so no need to sync.
BlkInfo *__blkcache_storage;

static if (N_CACHE_BLOCKS==1)
static if (N_CACHE_BLOCKS == 1)
{
version=single_cache;
}
Expand All @@ -43,47 +43,50 @@ else
{
import core.stdc.stdlib;
import core.stdc.string;
import core.thread.threadbase;
// allocate the block cache for the first time
immutable size = BlkInfo.sizeof * N_CACHE_BLOCKS;
__blkcache_storage = cast(BlkInfo *)malloc(size);
memset(__blkcache_storage, 0, size);
// use noscan, since we do not want to pin any cached allocations.
__blkcache_storage = cast(BlkInfo*) GC.calloc(size, BlkAttr.NO_SCAN);
ThreadBase.getThis().tlsGCData = __blkcache_storage;
}
return __blkcache_storage;
}

// called when thread is exiting.
static ~this()
/**
* Indicates whether an address has been marked by the GC.
*/
enum IsMarked : int
{
// free the blkcache
if (__blkcache_storage)
{
import core.stdc.stdlib;
free(__blkcache_storage);
__blkcache_storage = null;
}
no, /// Address is not marked.
yes, /// Address is marked.
unknown, /// Address is not managed by the GC.
}

alias IsMarkedDg = IsMarked delegate(void* addr) nothrow; /// The isMarked callback function.

// we expect this to be called with the lock in place
void processGCMarks(BlkInfo* cache, scope rt.tlsgc.IsMarkedDg isMarked) nothrow
void processGCMarks(void* data, scope IsMarkedDg isMarked) nothrow
{
if (!data)
return;

auto cache = cast(BlkInfo*)data;
thewilsonator marked this conversation as resolved.
Show resolved Hide resolved
// called after the mark routine to eliminate block cache data when it
// might be ready to sweep

debug(PRINTF) printf("processing GC Marks, %x\n", cache);
if (cache)
debug(PRINTF) foreach (i; 0 .. N_CACHE_BLOCKS)
{
debug(PRINTF) foreach (i; 0 .. N_CACHE_BLOCKS)
{
printf("cache entry %d has base ptr %x\tsize %d\tflags %x\n", i, cache[i].base, cache[i].size, cache[i].attr);
}
auto cache_end = cache + N_CACHE_BLOCKS;
for (;cache < cache_end; ++cache)
printf("cache entry %d has base ptr %x\tsize %d\tflags %x\n", i, cache[i].base, cache[i].size, cache[i].attr);
}
auto cache_end = cache + N_CACHE_BLOCKS;
for (;cache < cache_end; ++cache)
{
if (cache.base != null && isMarked(cache.base) == IsMarked.no)
{
if (cache.base != null && !isMarked(cache.base))
{
debug(PRINTF) printf("clearing cache entry at %x\n", cache.base);
cache.base = null; // clear that data.
}
debug(PRINTF) printf("clearing cache entry at %x\n", cache.base);
cache.base = null; // clear that data.
}
}
}
Expand All @@ -100,14 +103,14 @@ unittest
Get the cached block info of an interior pointer. Returns null if the
interior pointer's block is not cached.

NOTE: The base ptr in this struct can be cleared asynchronously by the GC,
NOTE: The following note was not valid, but is retained for historical
purposes. The data cannot be cleared because the stack contains a
reference to the affected block (e.g. through `interior`). Therefore,
the element will not be collected, and the data will remain valid.

ORIGINAL: The base ptr in this struct can be cleared asynchronously by the GC,
so any use of the returned BlkInfo should copy it and then check the
base ptr of the copy before actually using it.

TODO: Change this function so the caller doesn't have to be aware of this
issue. Either return by value and expect the caller to always check
the base ptr as an indication of whether the struct is valid, or set
the BlkInfo as a side-effect and return a bool to indicate success.
*/
BlkInfo *__getBlkInfo(void *interior) nothrow
{
Expand Down Expand Up @@ -151,6 +154,7 @@ void __insertBlkInfoCache(BlkInfo bi, BlkInfo *curpos) nothrow
version (single_cache)
{
*__blkcache = bi;
schveiguy marked this conversation as resolved.
Show resolved Hide resolved
return;
}
else
{
Expand Down
21 changes: 18 additions & 3 deletions druntime/src/core/internal/gc/impl/conservative/gc.d
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import core.gc.gcinterface;
import core.internal.container.treap;
import core.internal.spinlock;
import core.internal.gc.pooltable;
import core.internal.gc.blkcache;

import cstdlib = core.stdc.stdlib : calloc, free, malloc, realloc;
import core.stdc.string : memcpy, memset, memmove;
Expand Down Expand Up @@ -2873,7 +2874,7 @@ struct Gcx
markProcPid = 0;
// process GC marks then sweep
thread_suspendAll();
thread_processGCMarks(&isMarked);
thread_processTLSGCData(&clearBlkCacheData);
thread_resumeAll();
break;
case ChildStatus.running:
Expand Down Expand Up @@ -3108,7 +3109,7 @@ Lmark:
markAll!(markConservative!false)();
}

thread_processGCMarks(&isMarked);
thread_processTLSGCData(&clearBlkCacheData);
thread_resumeAll();
isFinal = false;
}
Expand Down Expand Up @@ -3161,13 +3162,27 @@ Lmark:
return freedPages;
}

/**
* Clear the block cache data if it exists, given the data which is the
* block info cache.
*
* Warning! This should only be called while the world is stopped inside
* the fullcollect function after all live objects have been marked, but
* before sweeping.
*/
void *clearBlkCacheData(void* data) scope nothrow
{
processGCMarks(data, &isMarked);
return data;
}

/**
* Returns true if the addr lies within a marked block.
*
* Warning! This should only be called while the world is stopped inside
* the fullcollect function after all live objects have been marked, but before sweeping.
*/
int isMarked(void *addr) scope nothrow
IsMarked isMarked(void *addr) scope nothrow
{
// first, we find the Pool this block is in, then check to see if the
// mark bit is clear.
Expand Down
6 changes: 3 additions & 3 deletions druntime/src/core/thread/osthread.d
Original file line number Diff line number Diff line change
Expand Up @@ -1171,7 +1171,7 @@ private extern (D) ThreadBase attachThread(ThreadBase _thisThread) @nogc nothrow
atomicStore!(MemoryOrder.raw)(thisThread.toThread.m_isRunning, true);
}
thisThread.m_isDaemon = true;
thisThread.tlsGCdataInit();
thisThread.tlsRTdataInit();
Thread.setThis( thisThread );

version (Darwin)
Expand Down Expand Up @@ -1246,15 +1246,15 @@ version (Windows)
if ( addr == GetCurrentThreadId() )
{
thisThread.m_hndl = GetCurrentThreadHandle();
thisThread.tlsGCdataInit();
thisThread.tlsRTdataInit();
Thread.setThis( thisThread );
}
else
{
thisThread.m_hndl = OpenThreadHandle( addr );
impersonate_thread(addr,
{
thisThread.tlsGCdataInit();
thisThread.tlsRTdataInit();
Thread.setThis( thisThread );
});
}
Expand Down
61 changes: 18 additions & 43 deletions druntime/src/core/thread/threadbase.d
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ private
alias ScanDg = void delegate(void* pstart, void* pend) nothrow;
alias rt_tlsgc_scan =
externDFunc!("rt.tlsgc.scan", void function(void*, scope ScanDg) nothrow);

alias rt_tlsgc_processGCMarks =
externDFunc!("rt.tlsgc.processGCMarks", void function(void*, scope IsMarkedDg) nothrow);
}


Expand Down Expand Up @@ -128,9 +125,14 @@ class ThreadBase
return (no_context || not_registered);
}

package void tlsGCdataInit() nothrow @nogc
ref void* tlsGCData() nothrow @nogc
{
m_tlsgcdata = rt_tlsgc_init();
return m_tlsgcdata;
}

package void tlsRTdataInit() nothrow @nogc
{
m_tlsrtdata = rt_tlsgc_init();
}

package void initDataStorage() nothrow
Expand All @@ -139,18 +141,18 @@ class ThreadBase

m_main.bstack = getStackBottom();
m_main.tstack = m_main.bstack;
tlsGCdataInit();
tlsRTdataInit();
}

package void destroyDataStorage() nothrow @nogc
{
rt_tlsgc_destroy(m_tlsgcdata);
m_tlsgcdata = null;
rt_tlsgc_destroy(m_tlsrtdata);
m_tlsrtdata = null;
}

package void destroyDataStorageIfAvail() nothrow @nogc
{
if (m_tlsgcdata)
if (m_tlsrtdata)
destroyDataStorage();
}

Expand Down Expand Up @@ -473,6 +475,7 @@ package(core.thread):
StackContext* m_curr;
bool m_lock;
private void* m_tlsgcdata;
private void* m_tlsrtdata;

///////////////////////////////////////////////////////////////////////////
// Thread Context and GC Scanning Support
Expand Down Expand Up @@ -1108,8 +1111,8 @@ private void scanAllTypeImpl(scope ScanAllThreadsTypeFn scan, void* curStackTop)
scanWindowsOnly(scan, t);
}

if (t.m_tlsgcdata !is null)
rt_tlsgc_scan(t.m_tlsgcdata, (p1, p2) => scan(ScanType.tls, p1, p2));
if (t.m_tlsrtdata !is null)
rt_tlsgc_scan(t.m_tlsrtdata, (p1, p2) => scan(ScanType.tls, p1, p2));
}
}

Expand Down Expand Up @@ -1159,43 +1162,15 @@ package void onThreadError(string msg) nothrow @nogc
}


/**
* Indicates whether an address has been marked by the GC.
*/
enum IsMarked : int
{
no, /// Address is not marked.
yes, /// Address is marked.
unknown, /// Address is not managed by the GC.
}

alias IsMarkedDg = int delegate(void* addr) nothrow; /// The isMarked callback function.
// GC-specific processing of TLSGC data.
alias ProcessTLSGCDataDg = void* delegate(void* data) nothrow;

/**
* This routine allows the runtime to process any special per-thread handling
* for the GC. This is needed for taking into account any memory that is
* referenced by non-scanned pointers but is about to be freed. That currently
* means the array append cache.
*
* Params:
* isMarked = The function used to check if $(D addr) is marked.
*
* In:
* This routine must be called just prior to resuming all threads.
*/
extern(C) void thread_processGCMarks(scope IsMarkedDg isMarked) nothrow
void thread_processTLSGCData(ProcessTLSGCDataDg dg) nothrow
{
for (ThreadBase t = ThreadBase.sm_tbeg; t; t = t.next)
{
/* Can be null if collection was triggered between adding a
* thread and calling rt_tlsgc_init.
*/
if (t.m_tlsgcdata !is null)
rt_tlsgc_processGCMarks(t.m_tlsgcdata, isMarked);
}
t.m_tlsgcdata = dg(t.m_tlsgcdata);
}


/**
* Returns the stack top of the currently active stack within the calling
* thread.
Expand Down
1 change: 1 addition & 0 deletions druntime/src/rt/lifetime.d
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ module rt.lifetime;
import core.attribute : weak;
import core.internal.array.utils : __arrayStart, __arrayClearPad;
import core.memory;
import core.internal.gc.blkcache;
debug(PRINTF) import core.stdc.stdio;
static import rt.tlsgc;

Expand Down
16 changes: 0 additions & 16 deletions druntime/src/rt/tlsgc.d
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ static import rt.lifetime, rt.sections;
struct Data
{
typeof(rt.sections.initTLSRanges()) tlsRanges;
rt.lifetime.BlkInfo** blockInfoCache;
}

/**
Expand All @@ -39,8 +38,6 @@ void* init() nothrow @nogc

// do module specific initialization
data.tlsRanges = rt.sections.initTLSRanges();
data.blockInfoCache = &rt.lifetime.__blkcache_storage;

return data;
}

Expand All @@ -67,16 +64,3 @@ void scan(void* data, scope ScanDg dg) nothrow
// do module specific marking
rt.sections.scanTLSRanges((cast(Data*)data).tlsRanges, dg);
}

alias int delegate(void* addr) nothrow IsMarkedDg;

/**
* GC sweep hook, called FOR each thread. Can be used to free
* additional thread local memory or associated data structures. Note
* that only memory allocated from the GC can have marks.
*/
void processGCMarks(void* data, scope IsMarkedDg dg) nothrow
{
// do module specific sweeping
rt.lifetime.processGCMarks(*(cast(Data*)data).blockInfoCache, dg);
}
Loading