Hotspot for BSD / opinion request

[David Holmes]

Hi David,

Sorry for the long lag on this. I've been looking into the mutex 
attribute defaults and it is unfortunate that POSIX allowed 
PTHREAD_MUTEX_DEFAULT to be an implementation specific alias for one of 
the other three mutex types: PTHREAD_MUTEX_NORMAL, 
PTHREAD_MUTEX_ERRORCHECK, PTHREAD_MUTEX_RECURSIVE. But even more 
unfortunate that FreeBSD decided to map it to ERRORCHECK instead of 
NORMAL as every other platform seems to do! (I note NetBSD defines a 
DEFAULT that is compatible with NORMAL, not ERRORCHECK.)

Fixing this for FreeBSD when it is not an officially supported platform 
is a little messy because we don't want to penalize all the other 
platforms with extra initialization code. Right now this could be 
confined to os_bsd.* but this code should be refactored into os_posix.* 
to be more widely shared.

That aside we do not need to keep an attr object per mutex as they will 
all share the same attributes. So we would have a single static instance 
and initialize it once during VM initialization, then use it when 
creating mutexes. Even then I would structure this so that the use of 
the attr object is hidden behind a macro so that it is a no-op on 
platforms that use NORMAL as the default.

I will file a RFE for sharing POSIX-based PlatformEvent/Parker code and 
include the mutex type issue as part of that.

Thanks,
David H.
-------


On 10/01/2017 3:37 AM, David CARLIER wrote:
> Hi sure thing
>
> diff --git a/src/os/bsd/vm/os_bsd.hpp b/src/os/bsd/vm/os_bsd.hpp
> --- a/src/os/bsd/vm/os_bsd.hpp
> +++ b/src/os/bsd/vm/os_bsd.hpp
> @@ -175,6 +175,7 @@
>    volatile int _Event;
>    volatile int _nParked;
>    pthread_mutex_t _mutex[1];
> +  pthread_mutexattr_t _attr[1];
>    pthread_cond_t  _cond[1];
>    double PostPad[2];
>    Thread * _Assoc;
> @@ -187,7 +188,11 @@
>      int status;
>      status = pthread_cond_init(_cond, NULL);
>      assert_status(status == 0, status, "cond_init");
> -    status = pthread_mutex_init(_mutex, NULL);
> +    status = pthread_mutexattr_init(_attr);
> +    assert_status(status == 0, status, "attr_init");
> +    status = pthread_mutexattr_settype(_attr, PTHREAD_MUTEX_NORMAL);
> +    assert_status(status == 0, status, "attr_settype");
> +    status = pthread_mutex_init(_mutex, _attr);
>      assert_status(status == 0, status, "mutex_init");
>      _Event   = 0;
>      _nParked = 0;
> @@ -206,6 +211,7 @@
>  class PlatformParker : public CHeapObj<mtInternal> {
>   protected:
>    pthread_mutex_t _mutex[1];
> +  pthread_mutexattr_t _attr[1];
>    pthread_cond_t  _cond[1];
>
>   public:       // TODO-FIXME: make dtor private
> @@ -216,7 +222,11 @@
>      int status;
>      status = pthread_cond_init(_cond, NULL);
>      assert_status(status == 0, status, "cond_init");
> -    status = pthread_mutex_init(_mutex, NULL);
> +    status = pthread_mutexattr_init(_attr);
> +    assert_status(status == 0, status, "attr_init");
> +    status = pthread_mutexattr_settype(_attr, PTHREAD_MUTEX_NORMAL);
> +    assert_status(status == 0, status, "attr_settype");
> +    status = pthread_mutex_init(_mutex, _attr);
>      assert_status(status == 0, status, "mutex_init");
>    }
>  };
>
>
>
> Kind regards.
>
> Thanks.
>
> On 9 January 2017 at 16:48, Daniel D. Daugherty
> <daniel.daugherty at oracle.com> wrote:
>> David C.,
>>
>> If you sent the patch as an attachment, then you should be advised that
>> the OpenJDK email servers strip attachments. If the patch is relatively
>> small, then you can send it in-line...
>>
>> Dan
>>
>>
>>
>> On 1/6/17 3:23 PM, David CARLIER wrote:
>>>
>>> Thanks, here a patch proposal. Kindest regards.
>>>
>>> On 24 December 2016 at 02:16, David Holmes <david.holmes at oracle.com>
>>> wrote:
>>>>
>>>> If there is a platform, we support, where the default settings are
>>>> inappropriate then setting them explicitly would be the right thing to
>>>> do.
>>>>
>>>> David
>>>>
>>>>
>>>> On 24/12/2016 10:23 AM, Vladimir Kozlov wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Forwarding to mailing lists since I can't answer.
>>>>> May be someone can answer your question on these lists.
>>>>>
>>>>> Regards,
>>>>> Vladimir
>>>>>
>>>>> On 12/23/16 2:17 AM, David CARLIER wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> this is my first mail to the maiing list, I have difficulties to push
>>>>>> messages to any mailing lists, but as BSD user/dev I was
>>>>>> looking at code's part like this one
>>>>>>
>>>>>>
>>>>>>
>>>>>> http://hg.openjdk.java.net/jdk9/dev/hotspot/file/70c6fae64754/src/os/bsd/vm/os_bsd.hpp
>>>>>>
>>>>>>
>>>>>> I was wondering if it would be better to explicitally set the attr
>>>>>> type to PTHREAD_MUTEX_NORMAL to insure consistency with AIX and Linux
>>>>>> (on FreeBSD it is PTHREAD_MUTEX_ERRORCHECK by default).
>>>>>>
>>>>>> What do you think ?
>>>>>>
>>>>>> Thanks in advance.
>>>>>>
>>>>>> Kindest regards.
>>>>>>
>>