RFR: JDK-8160350 cannot truss jdk9 [solaris]

[David Holmes]

On 2/07/2016 5:44 AM, Kim Barrett wrote:
>> On Jul 1, 2016, at 9:29 AM, Alan Burlison <Alan.Burlison at oracle.com> wrote:
>>
>> Summary:
>>
>> Microstate accounting has been always-on on Solaris since Solaris 9, the use of /proc/self/ctl to enable it is redundant and has the side-effect of breaking the use of truss on JVM processes. Microstate accounting code removed and other now-redundant *vtime* methods also removed.
>>
>> Tested with JPRT hostpot testset, all tests passed
>>
>> Webrev:   http://cr.openjdk.java.net/~coleenp/JDK-8160350/
>> Bug link: https://bugs.openjdk.java.net/browse/JDK-8160350
>>
>> --
>> Alan Burlison
>> --
>
> This doesn't look right to me.
>
> The BSD implementation of os::elapsedVTime presently just calls
> elapsedTime, with a comment "better than nothing, but not much".  I
> think it's worse than nothing, and should actually fail, with
> os::supports_vtime returning false for this platform.  Similarly,
> elapsedVtime on some other platforms falls back to elapsedTime, which
> I suspect is a mistake.

As I commented in the pre-review for this our use of elapsedVTime is 
currently broken because it is not always guarded with a check for VTime 
being enabled. If we added those checks then BSD/OSX could simply report 
that VTime is not supported and not enabled. Otherwise we can choose to 
unconditionally use elapsedVTime and it is simply a "best effort" 
approximation of thread-execution time. Alan took this latter path. 
Seems the general opinion now is to perhaps go the other path, or at 
least look more closely at the issue before deciding.

But one has to question what the code using elapsedVTime will do if 
VTime is not available? If it still has to report something and instead 
uses elapsedTime() then we may as well go first the path and have 
elapsedVTime be a best effort attempt which falls back to regular 
elapsedTime.

The main thing is to fix the truss issue on Solaris 11+ asap, so I'm 
okay with splitting this as described.

David
-----

> The problem is that the way one deals with vtime is different from the
> way one deals with elapsedTime values.  The predicate is needed so
> that code that wants to use vtime if available can be properly written
> to fall back to something else when it's not.  (Note that I think
> there might be bugs in how G1 is attempting to do that in some
> places.)
>
> I have a note to self to look into this and probably report some bugs,
> but haven't gotten around to doing so.
>
> So while we might no longer need os::enable_vtime and
> os::vtime_enabled, I think getting rid of os::supports_vtime isn't
> right.
>
> OTOH, the present usage of vtime is quite limited.  As noted, it's
> only used by G1, and there I think only for some logging / stats
> reporting.  Maybe we don't really need it at all?  I don't have an
> opinion on that, but other folks in the GC team (at least) might.
>