RFR: 8187042: Events to show which objects are associated with biased object revocations

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8187042: Events to show which objects are associated with biased object revocations

Robin Westberg
Hi all,

Please review this change to add event-based tracing events for biased lock revocations:

Webrev (courtesy of Erik Gahlin): http://cr.openjdk.java.net/~egahlin/8187042/

Best regards,
Robin
Reply | Threaded
Open this post in threaded view
|

RE: RFR: 8187042: Events to show which objects are associated with biased object revocations

Markus Gronlund

Hi Robin,

 

This looks good, thank you very much for adding this!

 

Cheers

Markus

 

From: Robin Westberg
Sent: den 6 oktober 2017 12:22
To: [hidden email]
Subject: RFR: 8187042: Events to show which objects are associated with biased object revocations

 

Hi all,

 

Please review this change to add event-based tracing events for biased lock revocations:

 

Webrev (courtesy of Erik Gahlin): http://cr.openjdk.java.net/~egahlin/8187042/

 

Best regards,

Robin

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8187042: Events to show which objects are associated with biased object revocations

Erik Gahlin
In reply to this post by Robin Westberg
Looks good!

Erik


On 6 Oct 2017, at 12:22, Robin Westberg <[hidden email]> wrote:

Hi all,

Please review this change to add event-based tracing events for biased lock revocations:

Webrev (courtesy of Erik Gahlin): http://cr.openjdk.java.net/~egahlin/8187042/

Best regards,
Robin

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8187042: Events to show which objects are associated with biased object revocations

David Holmes
In reply to this post by Robin Westberg
Hi Robin,

On 6/10/2017 8:22 PM, Robin Westberg wrote:
> Hi all,
>
> Please review this change to add event-based tracing events for biased
> lock revocations:
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8187042
> Webrev (courtesy of Erik Gahlin):
> http://cr.openjdk.java.net/~egahlin/8187042/

I have a few queries:

First, why is there no event for the self-revocation path?

Second, is there a reason you can't put the event management inside the
VM operation code and so avoid the need to adjust the safepoint counter?

Third, I would have expected to see more detail in the event such as
which thread (id) the object was biased to and which thread revoked the
bias. Even perhaps some notion of which instance was involved (though
that's harder to shows).

Thanks,
David

> Best regards,
> Robin
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8187042: Events to show which objects are associated with biased object revocations

Robin Westberg
Hi David,

Thanks for taking the time to look at this!

> On 9 Oct 2017, at 03:26, David Holmes <[hidden email]> wrote:
>
> Hi Robin,
>
> On 6/10/2017 8:22 PM, Robin Westberg wrote:
>> Hi all,
>> Please review this change to add event-based tracing events for biased lock revocations:
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8187042
>> Webrev (courtesy of Erik Gahlin): http://cr.openjdk.java.net/~egahlin/8187042/
>
> I have a few queries:
>
> First, why is there no event for the self-revocation path?

That’s a good question, I did not want to add events for revocations done without consulting the bulk revocation heuristics (as that could generate a very large amount of events), so I may have mistakenly thought of the self-revocation as a part of that.. But certainly makes sense to have an event there as well, I’ll do a bit of testing and add one.

> Second, is there a reason you can't put the event management inside the VM operation code and so avoid the need to adjust the safepoint counter?

Well yes, the event itself is configured to record the current thread together with a stack trace, but that requires that the event is actually generated from the thread that should be recorded.

> Third, I would have expected to see more detail in the event such as which thread (id) the object was biased to and which thread revoked the bias. Even perhaps some notion of which instance was involved (though that's harder to shows).

Right, I’ve been looking at capturing which thread the object was biased towards, but I was afraid of the possible races there as the thread pointer in the mark would have to be saved before executing the VM operation. For that to work 100% reliably I suspect it would have to be done inside the safepoint.

I will create an updated webrev after looking into adding an event for the self-revocation path.

Best regards,
Robin

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8187042: Events to show which objects are associated with biased object revocations

David Holmes
Hi Robin,

On 10/10/2017 12:57 AM, Robin Westberg wrote:

> Hi David,
>
> Thanks for taking the time to look at this!
>
>> On 9 Oct 2017, at 03:26, David Holmes <[hidden email]> wrote:
>>
>> Hi Robin,
>>
>> On 6/10/2017 8:22 PM, Robin Westberg wrote:
>>> Hi all,
>>> Please review this change to add event-based tracing events for biased lock revocations:
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8187042
>>> Webrev (courtesy of Erik Gahlin): http://cr.openjdk.java.net/~egahlin/8187042/
>>
>> I have a few queries:
>>
>> First, why is there no event for the self-revocation path?
>
> That’s a good question, I did not want to add events for revocations done without consulting the bulk revocation heuristics (as that could generate a very large amount of events), so I may have mistakenly thought of the self-revocation as a part of that.. But certainly makes sense to have an event there as well, I’ll do a bit of testing and add one.

Ok.

>> Second, is there a reason you can't put the event management inside the VM operation code and so avoid the need to adjust the safepoint counter?
>
> Well yes, the event itself is configured to record the current thread together with a stack trace, but that requires that the event is actually generated from the thread that should be recorded.

Ah - good point. And good to know the requesting thread is captured -
that wasn't clear to me from the event snippets in the webrev.

>> Third, I would have expected to see more detail in the event such as which thread (id) the object was biased to and which thread revoked the bias. Even perhaps some notion of which instance was involved (though that's harder to shows).
>
> Right, I’ve been looking at capturing which thread the object was biased towards, but I was afraid of the possible races there as the thread pointer in the mark would have to be saved before executing the VM operation. For that to work 100% reliably I suspect it would have to be done inside the safepoint.

Right the thread holding the bias may not even exist any more! This may
need to utilise the new Thread-SMR work (as a future RFE of course). :)

> I will create an updated webrev after looking into adding an event for the self-revocation path.

Thanks,
David

> Best regards,
> Robin
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8187042: Events to show which objects are associated with biased object revocations

Robin Westberg
Hi again,

Here’s an updated version that adds a separate event for the self-revocation path. It’s a new event class as it is a bit different from the non-self-revocation path, it does not have any relevant safepoint ID for example.

Webrev:

Third, I would have expected to see more detail in the event such as which thread (id) the object was biased to and which thread revoked the bias. Even perhaps some notion of which instance was involved (though that's harder to shows).
Right, I’ve been looking at capturing which thread the object was biased towards, but I was afraid of the possible races there as the thread pointer in the mark would have to be saved before executing the VM operation. For that to work 100% reliably I suspect it would have to be done inside the safepoint.

Right the thread holding the bias may not even exist any more! This may need to utilise the new Thread-SMR work (as a future RFE of course). :)

Ah yeah, that may be an effective way of doing it. Another idea suggested by Markus Grönlund was to capture the thread’s id inside the operation and propagate it through an additional field in the VM operation class. But anyway, I’ll file a separate RFE for investigating that improvement.

Best regards,
Robin

I will create an updated webrev after looking into adding an event for the self-revocation path.

Thanks,
David

Best regards,
Robin

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8187042: Events to show which objects are associated with biased object revocations

David Holmes
Thanks robin, these updates seem fine to me.

David

On 14/10/2017 12:56 AM, Robin Westberg wrote:

> Hi again,
>
> Here’s an updated version that adds a separate event for the
> self-revocation path. It’s a new event class as it is a bit different
> from the non-self-revocation path, it does not have any relevant
> safepoint ID for example.
>
> Webrev:
> http://cr.openjdk.java.net/~egahlin/8187042_2/
>
>>>> Third, I would have expected to see more detail in the event such as
>>>> which thread (id) the object was biased to and which thread revoked
>>>> the bias. Even perhaps some notion of which instance was involved
>>>> (though that's harder to shows).
>>> Right, I’ve been looking at capturing which thread the object was
>>> biased towards, but I was afraid of the possible races there as the
>>> thread pointer in the mark would have to be saved before executing
>>> the VM operation. For that to work 100% reliably I suspect it would
>>> have to be done inside the safepoint.
>>
>> Right the thread holding the bias may not even exist any more! This
>> may need to utilise the new Thread-SMR work (as a future RFE of
>> course). :)
>
> Ah yeah, that may be an effective way of doing it. Another idea
> suggested by Markus Grönlund was to capture the thread’s id inside the
> operation and propagate it through an additional field in the VM
> operation class. But anyway, I’ll file a separate RFE for investigating
> that improvement.
>
> Best regards,
> Robin
>
>>> I will create an updated webrev after looking into adding an event
>>> for the self-revocation path.
>>
>> Thanks,
>> David
>>
>>> Best regards,
>>> Robin
>
Reply | Threaded
Open this post in threaded view
|

RE: RFR: 8187042: Events to show which objects are associated with biased object revocations

Markus Gronlund
In reply to this post by Robin Westberg

Hi Robin,

 

Looks good.


Thanks

Markus

 

From: Robin Westberg
Sent: den 13 oktober 2017 16:56
To: David Holmes
Cc: [hidden email]
Subject: Re: RFR: 8187042: Events to show which objects are associated with biased object revocations

 

Hi again,

 

Here’s an updated version that adds a separate event for the self-revocation path. It’s a new event class as it is a bit different from the non-self-revocation path, it does not have any relevant safepoint ID for example.

 

Webrev:

 

Third, I would have expected to see more detail in the event such as which thread (id) the object was biased to and which thread revoked the bias. Even perhaps some notion of which instance was involved (though that's harder to shows).

Right, I’ve been looking at capturing which thread the object was biased towards, but I was afraid of the possible races there as the thread pointer in the mark would have to be saved before executing the VM operation. For that to work 100% reliably I suspect it would have to be done inside the safepoint.


Right the thread holding the bias may not even exist any more! This may need to utilise the new Thread-SMR work (as a future RFE of course). :)

 

Ah yeah, that may be an effective way of doing it. Another idea suggested by Markus Grönlund was to capture the thread’s id inside the operation and propagate it through an additional field in the VM operation class. But anyway, I’ll file a separate RFE for investigating that improvement.

 

Best regards,

Robin



I will create an updated webrev after looking into adding an event for the self-revocation path.


Thanks,
David


Best regards,
Robin

 

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8187042: Events to show which objects are associated with biased object revocations

Robin Westberg
Thanks for the reviews Markus, Erik and David!

Filed https://bugs.openjdk.java.net/browse/JDK-8189368 for the improvement to add information on the thread currently holding the bias when it is revoked.

Best regards,
Robin

On 16 Oct 2017, at 13:23, Markus Gronlund <[hidden email]> wrote:

Hi Robin,
 
Looks good.

Thanks
Markus
 
From: Robin Westberg 
Sent: den 13 oktober 2017 16:56
To: David Holmes
Cc: [hidden email]
Subject: Re: RFR: 8187042: Events to show which objects are associated with biased object revocations
 
Hi again,
 
Here’s an updated version that adds a separate event for the self-revocation path. It’s a new event class as it is a bit different from the non-self-revocation path, it does not have any relevant safepoint ID for example.
 
Webrev:
 
Third, I would have expected to see more detail in the event such as which thread (id) the object was biased to and which thread revoked the bias. Even perhaps some notion of which instance was involved (though that's harder to shows).
Right, I’ve been looking at capturing which thread the object was biased towards, but I was afraid of the possible races there as the thread pointer in the mark would have to be saved before executing the VM operation. For that to work 100% reliably I suspect it would have to be done inside the safepoint.

Right the thread holding the bias may not even exist any more! This may need to utilise the new Thread-SMR work (as a future RFE of course). :)
 
Ah yeah, that may be an effective way of doing it. Another idea suggested by Markus Grönlund was to capture the thread’s id inside the operation and propagate it through an additional field in the VM operation class. But anyway, I’ll file a separate RFE for investigating that improvement.
 
Best regards,
Robin


I will create an updated webrev after looking into adding an event for the self-revocation path.

Thanks,
David


Best regards,
Robin