precise GC without array support #1977

rainers · 2017-11-25T09:07:31Z

This is most #1603 without change of the GC interface.

Dynamic and associative arrays are scanned conservatively, though.

dlang-bot · 2017-11-25T09:07:32Z

Thanks for your pull request, @rainers!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

rainers · 2017-11-25T09:17:46Z

These are the benchmarks on my system:

jmh530 · 2018-01-09T23:01:52Z

@rainers Is there any way to leverage compile-time information that main is @safe? E.g.

enum bool isMainSafe = isSafe!main;

albeit without relying on stuff from std.traits.

Not that I follow everything in the current GC or your PR, but it looks like you are relying on run-time type information for some parts. I would think that if main is safe then you need to keep track of less information.

I had previously asked on the forum about the impact of @safe on the GC as that would disallow access of pointers in unions and certain casts. The response I received was that you would still need to know at run-time what parts of the code are @safe or not and the run-time doesn't have information about @safe. However, if main is safe, then you would know for sure that these casts aren't available, etc. For instance, functions in the garbage collector could have a static if statement on isMainSafe that changes behavior at compile-time.

rainers · 2018-01-10T07:57:08Z

Is there any way to leverage compile-time information that main is @safe?

There might be a little more information when knowing main is memory-safe (although I cannot think of any), but I don't think gurarantees are strong enough to really rely on for noticable optimizations:

there are too many escapes from safety, e.g. trusted or using C calling convention.
static constructors/destructors are still executed without being required safe.
there is no annotation of the stack for precise scanning
unions are rare enough to actually pose a big issue (this depends on the application, though)

jmh530 · 2018-01-10T16:13:14Z

@rainers I appreciate the reply.

I had considered @trusted. My thinking was to treat the trusted code as safe. Now that I think on it, you would probably need an additional switch to make clear that the assumption is being made. On C calling conventions, my assumption was that extern(C) would typically be @System so it would not be in @safe code except through @trusted blocks.

I had not considered static constructors/destructors, but they can also be labeled @safe (at least module ones, which I assume is what you meant), so I would imagine that they can also be checked if they are safe from the run-time, though I wouldn't know how.

I had known that precise stack scanning was an issue for D, but wasn't sure how it would be handled.

rainers · 2018-01-12T07:33:21Z

@jmh530 Can you show a concrete example how the assumption of @safe code might help the (precise) GC? (The implementation in this PR fills a bitmap of possible pointers when allocating memory).

jmh530 · 2018-01-12T14:51:40Z

@rainers No.

schveiguy · 2018-01-13T13:48:01Z

src/core/bitop.d

+     *   numBits - The total number of valid bits in the given bit array
+     *   startBit - The initial start index to start searching
+     */
+    this(const(size_t)* bitarr, size_t numBits, size_t startBit) @system


Hm.. this could be done I think with a default startBit arg instead of a new function.

I didn't want to change the performance or inlinability of the other ctor.

Possibly the inlinability may be affected, but it is the ctor and not the hot path. But it made me pause a bit to see so much duplicated code :)

The ctor can also be in a hot path. But you are right, it's not in the GC mark function. Actually, using the BitRange proved to be slower than testing single bits for possible pointers on my systems, so it's disabled via enum useBitRange = false;. @Jebbs measured improvements with the BitRange, though.

Actually, using the BitRange proved to be slower than testing single bits for possible pointers on my systems

I wrote the BitRange code to help me avoid the tedious task of finding all set bits in a bitset when checking for module cycles. I didn't spend really any time profiling, just used tricks I knew of, though it turned out to be faster than the previous method of foreaching over the bits and checking if they were set. Would be cool if we could improve it for your purposes. It probably depends highly on how dense the pointers are.

It probably depends highly on how dense the pointers are.

I guess so, too. The BitRange probably helps if it allows skipping a full cache line, but that doesn't seem to happen very often in our benchmarks.

schveiguy · 2018-01-13T13:49:15Z

src/gc/bits.d

        return core.bitop.bt(data, i);
    }

+    //pragma(inline,true)


Was this supposed to be commented out?

IIRC inlining didn't always work, but this might be fixed in the meantime. I'll put the pragma back in.

schveiguy · 2018-01-13T13:50:59Z

Can't do a full review, as I'm not steeped in how this all works, but I did a cursory look from a code perspective.

andralex · 2018-02-23T19:36:20Z

In wake of the recent work, we should improve the informativeness and reliability of GC benchmarks. @wilzbach sounds like a good project to put on your list?

n8sh · 2018-03-14T18:44:40Z

src/gc/impl/conservative/gc.d

+    }
+
+    pragma(inline,false)
+    void setPointerBitmap(void* p, size_t s, size_t allocSize, const TypeInfo ti, uint attr) nothrow


It may be clearer during reading and less prone to mistakes by future maintainers if these two functions were distinguished by having different names rather than just having the order of their last two arguments swapped.

rainers · 2018-12-27T18:20:38Z

closing in favor of #2418

rainers mentioned this pull request Nov 25, 2017

Add precise gc implementation #1603

Closed

dlang-bot added Needs Rebase needs a `git rebase` performed and removed Needs Rebase needs a `git rebase` performed labels Jan 1, 2018

wilzbach removed the Needs Rebase needs a `git rebase` performed label Jan 3, 2018

dlang-bot added Needs Rebase needs a `git rebase` performed Needs Work labels Jan 5, 2018

precise GC without array support

859867f

rainers force-pushed the gc_precise_no_emplace branch from 5689342 to 859867f Compare January 12, 2018 18:52

rainers requested review from Burgos, DmitryOlshansky, MartinNowak, WalterBright, andralex and schveiguy as code owners January 12, 2018 18:52

dlang-bot added Needs Work and removed Needs Work Needs Rebase needs a `git rebase` performed labels Jan 12, 2018

schveiguy reviewed Jan 13, 2018

View reviewed changes

gc.bits.set: force inlining

bee041d

dlang-bot removed the Needs Work label Jan 13, 2018

dlang-bot added the Needs Work label Jan 14, 2018

fix ddoc

f20b6ec

dlang-bot removed the Needs Work label Jan 14, 2018

n8sh reviewed Mar 14, 2018

View reviewed changes

rainers mentioned this pull request Nov 29, 2018

prepare for precise garbage collection sociomantic-tsunami/ocean#663

Merged

rainers added the GC garbage collector label Dec 24, 2018

rainers closed this Dec 27, 2018

Uh oh!

precise GC without array support #1977

precise GC without array support #1977

Uh oh!

Conversation

rainers commented Nov 25, 2017

Uh oh!

dlang-bot commented Nov 25, 2017

Bugzilla references

Uh oh!

rainers commented Nov 25, 2017

Uh oh!

jmh530 commented Jan 9, 2018

Uh oh!

rainers commented Jan 10, 2018

Uh oh!

jmh530 commented Jan 10, 2018

Uh oh!

rainers commented Jan 12, 2018

Uh oh!

jmh530 commented Jan 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

schveiguy commented Jan 13, 2018

Uh oh!

andralex commented Feb 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rainers commented Dec 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants