GC: remove unused field, fix gen spinlock alignment
AbandonedPublic

Authored by osa1 on Jun 27 2018, 2:28 AM.

Details

Summary

Remove unused failed_promotions field. Re-align sync field.

Previously sync was at offset 272, now it's at 192 (aligned to
64-bits).

osa1 created this revision.Jun 27 2018, 2:28 AM
osa1 added a comment.Jun 27 2018, 2:29 AM

I don't know how useful this alignment is. It seems like we don't align the generation itself so it seems to me that this alignment is useless. Am I missing anything?

simonmar requested changes to this revision.Jun 27 2018, 3:37 AM

The purpose of the padding is to ensure that sync is on a different cache line from some of the other fields, otherwise one thread spinning on the lock will interfere with other threads modifying other fields of the struct.

This revision now requires changes to proceed.Jun 27 2018, 3:37 AM
osa1 added a comment.Jun 27 2018, 4:12 AM

OK. What changes do you want exactly? The alignment was previously broken and I fixed it in this patch.

osa1 added a comment.Jun 27 2018, 4:13 AM

Ooh wait, I think I get it. The next field should also be aligned, correct?

So the padding is not there for alignment, it's there for cache line separation. I requested changes because I think this diff might be a regression (but this is complicated, different CPUs have different cache line sizes, and it's likely quite hard to measure the difference)

As pointed out by @simonmar, I don't think there is an issue here. The padding is merely to prevent false sharing. @osa1, if you don't disagree could you abandon this?

osa1 abandoned this revision.Jul 3 2018, 1:00 AM