Make the trace logger use js::Thread instead of PRThread

RESOLVED FIXED in Firefox 51

Status

()

Core
JavaScript Engine
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: fitzgen, Assigned: fitzgen)

Tracking

unspecified
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

This also introduces a hasher for js::Thread::Id.
Created attachment 8779066 [details] [diff] [review]
Make the trace logger use js::Thread instead of PRThread

This also introduces a hasher for js::Thread::Id which could certainly use a
good double checking. I wasn't totally sure which parts of the Id are actually
unique and/or belong in the hash.
Attachment #8779066 - Flags: review?(terrence)
Assignee: nobody → nfitzgerald
Blocks: 956899
Status: NEW → ASSIGNED
Comment on attachment 8779066 [details] [diff] [review]
Make the trace logger use js::Thread instead of PRThread

Review of attachment 8779066 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/threading/Thread.h
@@ +83,5 @@
> +    }
> +
> +    static void rekey(Id& key, const Id& newKey) {
> +      key = newKey;
> +    }

Please remove |rekey|. It should not be instantiated unless you try to actually use rekeying and we *really* want the compile to fail if you think you need to change a Thread's id.

::: js/src/threading/posix/Thread.cpp
@@ +39,5 @@
>  
> +/* static */ js::HashNumber
> +js::Thread::Hasher::hash(const Lookup& l)
> +{
> +  return mozilla::HashBytes(l.platformData()->ptThread, sizeof(pthread_t));

|ptThread| is a pthread_t, so you'll need an & in front of it to call HashBytes.
Attachment #8779066 - Flags: review?(terrence) → review+
Created attachment 8779512 [details] [diff] [review]
Make the trace logger use js::Thread instead of PRThread

Applied review feedback.
Attachment #8779512 - Flags: review+
Attachment #8779066 - Attachment is obsolete: true

Comment 5

2 years ago
Pushed by nfitzgerald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bf671f59015
Make the trace logger use js::Thread instead of PRThread; r=terrence

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5bf671f59015
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.