Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

testgc2: Make sure GC allocations are not optimized out. #1191

Merged
merged 2 commits into from

5 participants

@klickverbot
Collaborator

This makes sure the GC allocations are not optimized out, causing the assert to be hit, in addition to some cosmetic changes in a separate commit.

@klickverbot klickverbot reopened this
@klickverbot
Collaborator

Now just using a global variable. This is enough for LDC without link-time optimizations turned on to not optimize the allocations away, but of course not an ideal solution. If anybody has a better idea, please let me know.

@alexrp

Seems OK to me.

test/runnable/testgc2.d
@@ -27,14 +25,13 @@ void test1()
printf("This may take a while\n");
try
{
- byte[] b = new byte[size_t.max / 3];
- version (Windows)
- assert(0);
+ dummy = new byte[size_t.max / 3];
+ version (Windows)
+ assert(0);
@MartinNowak Collaborator

This shouldn't be Windows only, right?

@klickverbot Collaborator

To be honest, I have no idea why it is in there, so I just didn't touch it. Speculation mode on: Allocating size_t.max / 3 bytes might be possible on some 32 bit systems if they provide an uncluttered address space to applications, but maybe this is guaranteed to not happen on Windows?

@MartinNowak Collaborator

Ah, now you shed some light on this. Windows has only 3GB address space for 32 bit applications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@MartinNowak
Collaborator

This is enough for LDC without link-time optimizations turned on to not optimize the allocations away, but of course not an ideal solution. If anybody has a better idea, please let me know.

If it's not an actual problem then it's good for now. If LTO is really breaking this you could try to make it a shared variable and/or use atomicStore/asm.

@alexrp

You know, on second thought...

Why does this test even exist? Shouldn't it be in druntime, if anything? And even then, don't we have plenty of code virtually everywhere that tests this stuff? And besides, maintaining it seems to turn into a battle against the compiler's optimizer(s) which may behave differently over time.

Or am I being silly?

@donc
Collaborator

No, you're not being silly. Some of the tests in the test suite are very, very old (years older than D1.00), and date from long before the runtime even existed. Even now, there are very few tests in druntime. All these kind of tests definitely belong there. On the other hand, we currently have no mechanism for stand-alone tests for phobos and druntime.

@yebblies
Collaborator

Using the allocated data in a call to a runtime function (getCapacity?) would be a more robust way to prevent it from being optimized out.

@klickverbot
Collaborator

Rebased and changed the code to just printf the capacities of the allocated chunks to avoid optimizations.

@klickverbot
Collaborator

Ping?

@klickverbot
Collaborator

Rebased again. Should be entirely uncontroversial now (well, in theory a clever compiler could just set capacity to 0 and remove the allocations anyway, but I doubt this will ever be worth it).

@donc donc merged commit 80dc1d6 into from
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 18, 2012
  1. @klickverbot

    testgc2: Minor cleanup.

    klickverbot authored
    Tabs -> spaces, removed empty version(none) block.
  2. @klickverbot
This page is out of date. Refresh to see the latest.
Showing with 7 additions and 11 deletions.
  1. +7 −11 test/runnable/testgc2.d
View
18 test/runnable/testgc2.d
@@ -9,16 +9,12 @@ import core.exception : OutOfMemoryError;
void test1()
{
- version (none)
- {
- }
- else
- {
printf("This should not take a while\n");
try
{
- long[] l = new long[ptrdiff_t.max];
- assert(0);
+ long[] l = new long[ptrdiff_t.max];
+ printf("%lu\n", cast(ulong)l.capacity); // Make sure l is not optimized out.
+ assert(0);
}
catch (OutOfMemoryError o)
{
@@ -27,14 +23,14 @@ void test1()
printf("This may take a while\n");
try
{
- byte[] b = new byte[size_t.max / 3];
- version (Windows)
- assert(0);
+ byte[] b = new byte[size_t.max / 3];
+ printf("%lu\n", cast(ulong)b.capacity); // Make sure b is not optimized out.
+ version (Windows)
+ assert(0);
}
catch (OutOfMemoryError o)
{
}
- }
}
/*******************************************/
Something went wrong with that request. Please try again.