1151679 - Stream the property name of getprop and setprop optimization sites

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Shu-yu Guo [:shu]]

Simple quality of life improvement.

Comment 1

[Shu-yu Guo [:shu]]

Attachment: Stream the property name of getprop and setprop optimization sites.

Comment 2

[Kannan Vijayan [:djvj]]

Comment on attachment 8588854 [details] [diff] [review]
Stream the property name of getprop and setprop optimization sites.

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

::: js/src/jsfriendapi.cpp
@@ +1257,5 @@
> +JS_FRIEND_API(JSAtom*)
> +js::GetPropertyNameFromPC(JSScript* script, jsbytecode* pc)
> +{
> +    if (!IsGetPropPC(pc) && !IsSetPropPC(pc))
> +        return nullptr;

Should this be an ASSERT instead?  Ensuring that there _is_ an op with a propname at |pc|?

Comment 3

[Shu-yu Guo [:shu]]

(In reply to Kannan Vijayan [:djvj] from comment #2)
> Comment on attachment 8588854 [details] [diff] [review]
> Stream the property name of getprop and setprop optimization sites.
> 
> Review of attachment 8588854 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jsfriendapi.cpp
> @@ +1257,5 @@
> > +JS_FRIEND_API(JSAtom*)
> > +js::GetPropertyNameFromPC(JSScript* script, jsbytecode* pc)
> > +{
> > +    if (!IsGetPropPC(pc) && !IsSetPropPC(pc))
> > +        return nullptr;
> 
> Should this be an ASSERT instead?  Ensuring that there _is_ an op with a
> propname at |pc|?

No, since the pc given by ForEachTrackedOptimizationAttempt could be a GET/SETELEM or a CALL or something else. We shouldn't expose friendapi functions that test for the opcode of the *pc, so that's why that function tests for IsGetPropPC || IsSetPropPC.

Comment 4

[Shu-yu Guo [:shu]]

Attachment: Display the property name in the JIT Optimizations side pane.

Comment 5

[Shu-yu Guo [:shu]]

Attachment: Display the property name in the JIT Optimizations side pane.

Comment 6

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8589429 [details] [diff] [review]
Display the property name in the JIT Optimizations side pane.

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

The ellipses masking should be handled via CSS, but I was having a lot of trouble myself getting the whole pane rendered correctly, and working with many different length strings. Seeing how this panel will probably not stay the way it is for very long, this is fine IMO.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4fa628562f6f
https://hg.mozilla.org/mozilla-central/rev/9fc22f95a34b