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

Avoid GC allocation when building the search path #3391

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Avoid GC allocation when building the search path #3391

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e0dfd69
Avoid GC allocation when building the search path
ryuukk Mar 10, 2021
73eefd6
Update src/core/sys/windows/stacktrace.d
ryuukk Mar 10, 2021
2b53cbf
Use heap allocated array instead of a stack one to retrieve paths
ryuukk Mar 10, 2021
745f975
Fix build
ryuukk Mar 10, 2021
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
35 changes: 20 additions & 15 deletions src/core/sys/windows/stacktrace.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,26 +345,28 @@ extern(Windows) BOOL FixupDebugHeader(HANDLE hProcess, ULONG ActionCode,
return FALSE;
}

private string generateSearchPath()
private size_t generateSearchPath(char[] path)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private size_t generateSearchPath(char[] path)
private size_t generateSearchPath(ref char[MAX_PATH] path)

Otherwise one could pass null here.

Copy link
Member

@n8sh n8sh Mar 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This size is wrong: the "search path" being generated is a concatenation of semicolon-separated paths each of which individually has (EDIT: presumably) that path limit.

Copy link
Member

Choose a reason for hiding this comment

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

Then we can't really avoid GC allocations, can we ? We could however use malloc.

Copy link
Author

Choose a reason for hiding this comment

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

I made some changes after reading your messages, it is now done in 2 pass, 1st to get the size needed, then i create the array with malloc, i don't know if that is how it should be done

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess a reasonable stack size would be char[MAX_PATH * defaultPathList.length] temp. Doing it in a single pass and only resorting to a heap allocation + 2nd pass in case that size doesn't suffice (very unlikely) would be ideal.

{
__gshared string[3] defaultPathList = ["_NT_SYMBOL_PATH",
"_NT_ALTERNATE_SYMBOL_PATH",
"SYSTEMROOT"];
__gshared string[3] defaultPathList = [
"_NT_SYMBOL_PATH", "_NT_ALTERNATE_SYMBOL_PATH", "SYSTEMROOT"
];

string path;
char[2048] temp = void;
char[MAX_PATH] temp = void;
DWORD len;

foreach ( e; defaultPathList )
size_t index = 0;
foreach (e; defaultPathList)
{
if ( (len = GetEnvironmentVariableA( e.ptr, temp.ptr, temp.length )) > 0 )
if ((len = GetEnvironmentVariableA(e.ptr, temp.ptr, temp.length)) > 0)
{
path ~= temp[0 .. len];
path ~= ";";
path[index .. index + len] = temp[0 .. len];
index += len;
path[index] = ';';
index += 1;
}
}
path ~= "\0";
return path;
path[index] = '\0';
index += 1;
return index;
}


Expand Down Expand Up @@ -400,9 +402,12 @@ shared static this()
symOptions |= SYMOPT_DEFERRED_LOAD;
symOptions = dbghelp.SymSetOptions( symOptions );

debug(PRINTF) printf("Search paths: %s\n", generateSearchPath().ptr);
char[MAX_PATH] buffer;
generateSearchPath(buffer);

debug(PRINTF) printf("Search paths: %s\n", buffer.ptr);

if (!dbghelp.SymInitialize(hProcess, generateSearchPath().ptr, TRUE))
if (!dbghelp.SymInitialize(hProcess, buffer.ptr, TRUE))
return;

dbghelp.SymRegisterCallback64(hProcess, &FixupDebugHeader, 0);
Expand Down