Skip to content

Commit

Permalink
Refine comment.
Browse files Browse the repository at this point in the history
  • Loading branch information
@ltratt
ltratt committed Dec 5, 2020
1 parent 63ce9d5 commit e65104e
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions ykrt/src/mt.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,9 @@ impl MTThread {
/// The innards of a meta-tracer thread.
struct MTThreadInner {
mt: MT,
/// A value that uniquely identifies a thread and that is guaranteed not to overlap with
/// PHASE_TAG. For simplicities sake this is a pointer to a malloc'd chunk of memory.
/// A value that uniquely identifies a thread. For simplicities sake this is a pointer to a
/// malloc'd chunk of memory since that pointer's alignment means that we won't conflict with
/// the bits set in PHASE_TAG.

This comment has been minimized.

Sign in to view

Copy link
@vext01

vext01 Dec 5, 2020

Contributor

How about:

A value that uniquely identifies a thread. For simplicity (and portability) this is a pointer to a malloc'd byte. The pointer's alignment means that it won't conflict with the bits in PHASE_TAG.

All of this talk of alignment got me reading the malloc(3) man pages. The alignment guarantees are vague, or at least oddly worded on both Linux and OpenBSD.

Linux:

The malloc() and calloc() functions return a pointer to the allocated memory, which is suitably aligned for any built-in type.

OpenBSD:

The allocated space is suitably aligned (after possible pointer coercion) for storage of any type of object.

So does that mean word-aligned?

What is the alignment of the pointer returned when we ask to allocate one byte?

[I also found alloc_aligned(3) which does an allocation at an alignment of your choosing. Could be useful in the future?]

This comment has been minimized.

Sign in to view

Copy link
@ltratt

ltratt Dec 5, 2020

Author Contributor

So does that mean word-aligned?

I also reread malloc this morning (before your comment :) ) and worried for about two seconds because you're right that the guarantee is very weak. There are platforms where non-word-aligned things are OK and where we wouldn't necessarily have to receive non-word-aligned memory. But we might as well be practical here: a malloc which returned non-word-aligned things is going to break so much software that it's exceedingly unlikely to make it beyond a first experiment. The "proper" solution, I suspect, is to call Rust's malloc with a specific alignment, but I think that might be worth doing in a subsequent PR.

FWIW, I think we're bikeshedding on the actual comment at this point

This comment has been minimized.

Sign in to view

Copy link
@ltratt

ltratt Dec 5, 2020

Author Contributor

On reflection, maybe it would be worth doing the aligned-malloc thing in this PR. I'll take a look at that later today.

This comment has been minimized.

Sign in to view

Copy link
@vext01

vext01 Dec 5, 2020

Contributor

On reflection...

OK

FWIW, I think we're bikeshedding on the actual comment at this point

It's just that I had problems parsing it. It gets jarring near"since", where I initially thought there was a missing full-stop, but reading on, that can't be the case.

This comment has been minimized.

Sign in to view

Copy link
@bjorn3

bjorn3 Dec 5, 2020

Collaborator

Posix has a function for this. I believe it is called posix_memalign.
tid: *mut u8,
hot_threshold: HotThreshold,
#[allow(dead_code)]
Expand Down

0 comments on commit e65104e

Please sign in to comment.