Remove precise box allocation #36

jacob-hughes · 2021-05-06T22:44:40Z

This removes precise box allocation as it can't currently be implemented correctly due to limitations with Boehm. However, rustgc will now default to trying to allocate things as untraceable to reduce mark-time wherever possible. In longer running benchmarks on yksom, this gives us some modest performance improvement:

----------------------------------------------------------------------------------------
  Benchmark     Executor      Suite   Extra   Core   Size   Var   #Samples   Mean (ms)
----------------------------------------------------------------------------------------
  Bounce        baseline      micro       2      1                     100   91
  Bounce        untraceable   micro       2      1                     100   105
  BubbleSort    baseline      micro       3      1                     100   146
  BubbleSort    untraceable   micro       3      1                     100   142
  DeltaBlue     baseline      macro      50      1                      10   91
  DeltaBlue     untraceable   macro      50      1                      10   105
  Dispatch      baseline      micro       2      1                     100   71
  Dispatch      untraceable   micro       2      1                     100   75
  Fannkuch      baseline      micro       6      1                     100   64
  Fannkuch      untraceable   micro       6      1                     100   61
  Fibonacci     baseline      micro       3      1                     100   270
  Fibonacci     untraceable   micro       3      1                     100   266
  FieldLoop     baseline      micro       1      1                     100   51
  FieldLoop     untraceable   micro       1      1                     100   43
  IntegerLoop   baseline      micro       2      1                     100   135
  IntegerLoop   untraceable   micro       2      1                     100   130
  JsonSmall     baseline      macro      10      1                      10   1316
  JsonSmall     untraceable   macro      10      1                      10   1261
  List          baseline      micro       2      1                     100   131
  List          untraceable   micro       2      1                     100   125
  Loop          baseline      micro       5      1                     100   188
  Loop          untraceable   micro       5      1                     100   188
  Mandelbrot    baseline      micro      30      1                     100   146
  Mandelbrot    untraceable   micro      30      1                     100   138
  NBody         baseline      macro     500      1                      10   72
  NBody         untraceable   macro     500      1                      10   59
  PageRank      baseline      macro      40      1                      10   157
  PageRank      untraceable   macro      40      1                      10   124
  Permute       baseline      micro       3      1                     100   112
  Permute       untraceable   micro       3      1                     100   108
  Queens        baseline      micro       2      1                     100   84
  Queens        untraceable   micro       2      1                     100   84
  QuickSort     baseline      micro       1      1                     100   71
  QuickSort     untraceable   micro       1      1                     100   67
  Recurse       baseline      micro       3      1                     100   97
  Recurse       untraceable   micro       3      1                     100   94
  Sieve         baseline      micro       4      1                     100   358
  Sieve         untraceable   micro       4      1                     100   326
  Storage       baseline      micro       1      1                     100   70
  Storage       untraceable   micro       1      1                     100   62
  Sum           baseline      micro       2      1                     100   70
  Sum           untraceable   micro       2      1                     100   67
  Towers        baseline      micro       2      1                     100   129
  Towers        untraceable   micro       2      1                     100   131
  TreeSort      baseline      micro       1      1                     100   182
  TreeSort      untraceable   micro       1      1                     100   189
  WhileLoop     baseline      micro      10      1                     100   171
  WhileLoop     untraceable   micro      10      1                     100   168

Unfortunately, yksom makes use of the Gc::new_from_layout pattern quite a bit, which currently still allocates everything conservatively. We might see even better results once that is also changed to use this approach.

ltratt · 2021-05-07T07:19:20Z

library/alloc/src/alloc.rs

+    ) -> Result<NonNull<[u8]>, AllocError> {
+        match layout.size() {
+            0 => Ok(NonNull::slice_from_raw_parts(layout.dangling(), 0)),
+            // SAFETY: `layout` is non-zero in size,


I'm not sure what this comment is supposed to be attached to (presumably the second clause?).

rustc has a lint which won't allow an unsafe block without a comment explaining why the unsafe block is ok to use.

Oh! Would it be clearer to write this as:

x => y { // comment unsafe { ... } }

?

I agree this is better and I went to change it, but then x.py fmt reverted my changes. I think we should leave it because fighting with the formatter is just going to cause pain.

If it reverts it, I don't think we have much choice :)

ltratt · 2021-05-07T07:20:41Z

However, rustgc will now default to trying to allocate things as untraceable to reduce mark-time wherever possible.

I'm not sure I understand the implications of this: as written it sounds like it might cause uncorrectness? I'm sure I'm wrong, but I'd appreciate a deeper explanation :)

jacob-hughes · 2021-05-07T09:01:57Z

I'm sure I'm wrong, but I'd appreciate a deeper explanation :)

This will make use of the NoTrace auto trait to work out whether something should allocate untraceable or not. If a type T implements NoTrace, then it will be allocated untraceable. This is familiar (and used to be enabled), but I temporarily disabled everything apart from conservative allocation while I tried fixing issues with the alloc precise stuff.

In yksom, this change alone leads to incorrectness because yksom sometimes treats usizes as pointers. However, it's safe to use now because of this change. From now on, this change will only be incorrect when used with applications which type pun pointers in some way. If that's the case, those applications need to inform GC by negatively implementing NoTrace for those types (or disable this optimization entirely with -C gc_precise_marking=false)

ltratt · 2021-05-07T09:03:22Z

OK, I'm with you. So, to be clear, the default is now "usizes are not treated as potential pointers unless you explicitly tell us that they are"?

jacob-hughes · 2021-05-07T09:08:02Z

That's correct. However *mut T and *const T are still treated as potential pointers to GC values even if T impls NoTrace. For example:

struct S {
   inner: *mut u8
}

S.inner would be scanned by the GC. Personally I think this is a bit too conservative, and it should be the responsibility of the user to tell the GC that this is, in fact, a pointer to a Gc value. But I'm happy to leave this as is for now.

ltratt · 2021-05-07T09:12:40Z

I agree that if a struct is declared NoTrace that should mean "don't even scan pointers in here".

I'm fine with the PR, modulo the comment about possibly sort-of reformatting the unsafe block.

ltratt · 2021-05-07T09:14:01Z

bors r+

bors · 2021-05-07T09:35:52Z

Build failed:

buildbot/buildbot-build-script

jacob-hughes · 2021-05-10T11:10:40Z

bors try

bors · 2021-05-10T11:35:24Z

try

Build failed:

buildbot/buildbot-build-script

jacob-hughes · 2021-05-10T12:00:15Z

bors try

bors · 2021-05-10T12:22:47Z

try

Build failed:

buildbot/buildbot-build-script

jacob-hughes · 2021-05-10T12:25:12Z

bors try

bors · 2021-05-10T13:05:14Z

try

Build succeeded:

buildbot/buildbot-build-script

jacob-hughes · 2021-05-10T13:54:59Z

This can be squashed now when you're ready.

ltratt · 2021-05-10T13:56:42Z

Please squash.

There is currently no support for non-GC'd precise heap allocation in Boehm. Until support for this is added, it is not correct to allocate non-garbage-collectable memory this way.

This is a hot path for all of Rust's box allocation. For some reason, I implemented this with enum pattern matching, which caused unnecessary branching in such a performance critical part of the allocator.

jacob-hughes · 2021-05-10T13:58:03Z

Squashed

ltratt · 2021-05-10T13:58:21Z

bors r+

bors · 2021-05-10T14:37:15Z

Build succeeded:

buildbot/buildbot-build-script

jacob-hughes assigned ltratt May 6, 2021

ltratt reviewed May 7, 2021

View reviewed changes

jacob-hughes added 5 commits May 10, 2021 14:57

Remove precise allocation for Box

af3aaf3

There is currently no support for non-GC'd precise heap allocation in Boehm. Until support for this is added, it is not correct to allocate non-garbage-collectable memory this way.

Turn on untraceable allocation by default

711d75e

Remove runtime branch during allocation

4928862

This is a hot path for all of Rust's box allocation. For some reason, I implemented this with enum pattern matching, which caused unnecessary branching in such a performance critical part of the allocator.

Expand types which can be safely allocated as untraceable

fffcbdc

Make Gc::new() alloc untraceable by default

45f42b7

jacob-hughes force-pushed the fix_precise_allocation branch from 004ef38 to 45f42b7 Compare May 10, 2021 13:57

bors bot merged commit e234c1c into softdevteam:master May 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove precise box allocation #36

Remove precise box allocation #36

jacob-hughes commented May 6, 2021

ltratt May 7, 2021

jacob-hughes May 7, 2021

ltratt May 7, 2021

jacob-hughes May 7, 2021

ltratt May 7, 2021

ltratt commented May 7, 2021

jacob-hughes commented May 7, 2021

ltratt commented May 7, 2021

jacob-hughes commented May 7, 2021 •

edited

Loading

ltratt commented May 7, 2021

ltratt commented May 7, 2021

bors bot commented May 7, 2021

jacob-hughes commented May 10, 2021

bors bot commented May 10, 2021

jacob-hughes commented May 10, 2021

bors bot commented May 10, 2021

jacob-hughes commented May 10, 2021

bors bot commented May 10, 2021

jacob-hughes commented May 10, 2021

ltratt commented May 10, 2021

jacob-hughes commented May 10, 2021

ltratt commented May 10, 2021

bors bot commented May 10, 2021

Remove precise box allocation #36

Remove precise box allocation #36

Conversation

jacob-hughes commented May 6, 2021

ltratt May 7, 2021

Choose a reason for hiding this comment

jacob-hughes May 7, 2021

Choose a reason for hiding this comment

ltratt May 7, 2021

Choose a reason for hiding this comment

jacob-hughes May 7, 2021

Choose a reason for hiding this comment

ltratt May 7, 2021

Choose a reason for hiding this comment

ltratt commented May 7, 2021

jacob-hughes commented May 7, 2021

ltratt commented May 7, 2021

jacob-hughes commented May 7, 2021 • edited Loading

ltratt commented May 7, 2021

ltratt commented May 7, 2021

bors bot commented May 7, 2021

jacob-hughes commented May 10, 2021

bors bot commented May 10, 2021

try

jacob-hughes commented May 10, 2021

bors bot commented May 10, 2021

try

jacob-hughes commented May 10, 2021

bors bot commented May 10, 2021

try

jacob-hughes commented May 10, 2021

ltratt commented May 10, 2021

jacob-hughes commented May 10, 2021

ltratt commented May 10, 2021

bors bot commented May 10, 2021

jacob-hughes commented May 7, 2021 •

edited

Loading