[Mono-dev] SetThreadPriority patch for mono-3.2.8

[David Nelson]

It is good to show that you have tested your code; it would be even better
to add tests to the Mono test suite to serve as a regression test.

Also, I could be wrong about this, but I think there is a potential problem
lurking in the new thread_priority field in _MonoInternalThread. It is
declared as type "int", which in C (unlike C#) is not guaranteed to be of
any particular size (it is usually the same as the system word size,
meaning it would be 32 bits on a 32-bit architecture and 64 bits on a
64-bit architecture; but even that is not guaranteed). This would mean that
Thread and _MonoInternalThread would not be the same size on 64-bit
architectures. Changing thread_priority in _MonoInternalThread from int to
gint32 would guarantee that the field is always 32 bits, matching Thread.

On Thu, Nov 6, 2014 at 9:38 AM, Bent Bisballe Nyeng <deva at aasimon.org>
wrote:

> Hi Andres
>
> I have added both test code and results as a comment to the pull request.
>
> Kind regards
> Bent Bisballe Nyeng
>
> On 11/06/14 15:22, Andres G. Aragoneses wrote:
>
>> Hey Bisballe,
>>
>> Thanks for putting your work as a pull request!
>>
>> It would be nice if you could include in it some unit tests that
>> demonstrate that the feature is working correctly (this will probably
>> also make the PR more likely to be reviewed/merged soon). You can find
>> an example of such tests here:
>>
>> https://bugzilla.novell.com/show_bug.cgi?id=540524
>>
>> Thanks
>>
>> On 06/11/14 15:16, Bent Bisballe Nyeng wrote:
>>
>>> I have made pull request now: https://github.com/mono/mono/pull/1391
>>>
>>> I have updated the patch to work with HEAD and tested it. Everything
>>> seems to work as expected.
>>>
>>> I'm a bit new to the whole github concept, so forgive me if I have not
>>> done everything by the book ;-)
>>>
>>> Kind regards
>>> Bent Bisballe Nyeng
>>>
>>> On 11/06/14 12:37, Alexander Köplinger wrote:
>>>
>>>> There is a PR that also claims to implement SetThreadPriority
>>>> (https://github.com/mono/mono/pull/1272), but it has many other
>>>> unrelated changes, so not in a state to be merged.
>>>>  From a quick look, your patch seems to be much more focused and thus
>>>> more likely to get merged. Can you open a pull request on GitHub?
>>>>
>>>> -- Alex
>>>>
>>>>
>>>>  > Date: Thu, 6 Nov 2014 09:12:02 +0100
>>>>  > From: deva at aasimon.org
>>>>  > To: mono-devel-list at lists.ximian.com
>>>>  > Subject: [Mono-dev] SetThreadPriority patch for mono-3.2.8
>>>>  >
>>>>  > Hi mono devs
>>>>  >
>>>>  > I created a patch for SetThreadPriority support in mono-3.2.8 and
>>>> would
>>>>  > very much like som feedback on it.
>>>>  > It can be found here:
>>>>  > https://gist.github.com/aasimon/c8ae6fc3cf5d9b82b6ca
>>>>  > Comments are welcome both here on the list as well as on the actual
>>>> gist
>>>>  > paste.
>>>>  >
>>>>  > Kind regards
>>>>  > Bent Bisballe Nyeng
>>>>  > _______________________________________________
>>>>  > Mono-devel-list mailing list
>>>>  > Mono-devel-list at lists.ximian.com
>>>>  > http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>
>>>
>>
>> _______________________________________________
>> Mono-devel-list mailing list
>> Mono-devel-list at lists.ximian.com
>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ximian.com/pipermail/mono-devel-list/attachments/20141106/90a0f8b2/attachment.html>