<div dir="ltr"><div><div>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.<br><br></div>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.<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Nov 6, 2014 at 9:38 AM, Bent Bisballe Nyeng <span dir="ltr"><<a href="mailto:deva@aasimon.org" target="_blank">deva@aasimon.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Andres<br>
<br>
I have added both test code and results as a comment to the pull request.<span class="im HOEnZb"><br>
<br>
Kind regards<br>
Bent Bisballe Nyeng<br>
<br></span><div class="HOEnZb"><div class="h5">
On 11/06/14 15:22, Andres G. Aragoneses wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hey Bisballe,<br>
<br>
Thanks for putting your work as a pull request!<br>
<br>
It would be nice if you could include in it some unit tests that<br>
demonstrate that the feature is working correctly (this will probably<br>
also make the PR more likely to be reviewed/merged soon). You can find<br>
an example of such tests here:<br>
<br>
<a href="https://bugzilla.novell.com/show_bug.cgi?id=540524" target="_blank">https://bugzilla.novell.com/<u></u>show_bug.cgi?id=540524</a><br>
<br>
Thanks<br>
<br>
On 06/11/14 15:16, Bent Bisballe Nyeng wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I have made pull request now: <a href="https://github.com/mono/mono/pull/1391" target="_blank">https://github.com/mono/mono/<u></u>pull/1391</a><br>
<br>
I have updated the patch to work with HEAD and tested it. Everything<br>
seems to work as expected.<br>
<br>
I'm a bit new to the whole github concept, so forgive me if I have not<br>
done everything by the book ;-)<br>
<br>
Kind regards<br>
Bent Bisballe Nyeng<br>
<br>
On 11/06/14 12:37, Alexander Köplinger wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
There is a PR that also claims to implement SetThreadPriority<br>
(<a href="https://github.com/mono/mono/pull/1272" target="_blank">https://github.com/mono/mono/<u></u>pull/1272</a>), but it has many other<br>
unrelated changes, so not in a state to be merged.<br>
 From a quick look, your patch seems to be much more focused and thus<br>
more likely to get merged. Can you open a pull request on GitHub?<br>
<br>
-- Alex<br>
<br>
<br>
 > Date: Thu, 6 Nov 2014 09:12:02 +0100<br>
 > From: <a href="mailto:deva@aasimon.org" target="_blank">deva@aasimon.org</a><br>
 > To: <a href="mailto:mono-devel-list@lists.ximian.com" target="_blank">mono-devel-list@lists.ximian.<u></u>com</a><br>
 > Subject: [Mono-dev] SetThreadPriority patch for mono-3.2.8<br>
 ><br>
 > Hi mono devs<br>
 ><br>
 > I created a patch for SetThreadPriority support in mono-3.2.8 and<br>
would<br>
 > very much like som feedback on it.<br>
 > It can be found here:<br>
 > <a href="https://gist.github.com/aasimon/c8ae6fc3cf5d9b82b6ca" target="_blank">https://gist.github.com/<u></u>aasimon/c8ae6fc3cf5d9b82b6ca</a><br>
 > Comments are welcome both here on the list as well as on the actual<br>
gist<br>
 > paste.<br>
 ><br>
 > Kind regards<br>
 > Bent Bisballe Nyeng<br>
 > ______________________________<u></u>_________________<br>
 > Mono-devel-list mailing list<br>
 > <a href="mailto:Mono-devel-list@lists.ximian.com" target="_blank">Mono-devel-list@lists.ximian.<u></u>com</a><br>
 > <a href="http://lists.ximian.com/mailman/listinfo/mono-devel-list" target="_blank">http://lists.ximian.com/<u></u>mailman/listinfo/mono-devel-<u></u>list</a><br>
</blockquote></blockquote>
<br>
<br>
______________________________<u></u>_________________<br>
Mono-devel-list mailing list<br>
<a href="mailto:Mono-devel-list@lists.ximian.com" target="_blank">Mono-devel-list@lists.ximian.<u></u>com</a><br>
<a href="http://lists.ximian.com/mailman/listinfo/mono-devel-list" target="_blank">http://lists.ximian.com/<u></u>mailman/listinfo/mono-devel-<u></u>list</a><br>
</blockquote>
<br>
______________________________<u></u>_________________<br>
Mono-devel-list mailing list<br>
<a href="mailto:Mono-devel-list@lists.ximian.com" target="_blank">Mono-devel-list@lists.ximian.<u></u>com</a><br>
<a href="http://lists.ximian.com/mailman/listinfo/mono-devel-list" target="_blank">http://lists.ximian.com/<u></u>mailman/listinfo/mono-devel-<u></u>list</a><br>
</div></div></blockquote></div><br></div>