Setting different fields of anonymous object from separate threads about hxcpp HOT 5 CLOSED

Yes, generally you are on your own when it comes to locking of structures.
However, I agree you should be able to access (but not create or delete)
items from a hash without a lock. I think the problem here is that the ++
is generating a "set" command and even though the item exists and therefor
no insert will happen, it still iterates over the tree (and mutates it by
the look of the code - I did not write it). I think the best solution
would be for an optimised no-mutate path for the case of when the value is
already in the tree. This should save some allocs too.

On Mon, Feb 17, 2014 at 10:44 PM, maxless [email protected] wrote:

The following example while technically does not have any racing
conditions, produces segfault in HXCPP on Linux 64-bit. It works fine in
Neko environment.

#if neko
import neko.vm.Thread;
#elseif cpp
import cpp.vm.Thread;
#end

class Test
{
var anon: { v1: Int, v2: Int };

public function new()
{
anon = { v1: 0, v2: 0 };
var t1 = Thread.create(t1run);
var t2 = Thread.create(t2run);
}

function t1run()
{
while (true)
anon.v1++;
}

function t2run()
{
while (true)
anon.v2++;
}

public static function main()
{
var t = new Test();
while (true)
Sys.sleep(1);
}
}

Running it through GDB shows that the segfault is in one of the lines in
RBTree implementation (I'm using the latest git version currently, switched
over from using 3.0.2 because of another bug):

0x0000000000477aa6 in RBTree<String, Dynamic>::Insert (this=0x7ffff7ee5a44, inKey=..., inValue=...) at /home/maxless/svn/hxcpp/src/hx/RedBlack.h:159
159 root->red = 0;

0x0000000000477a35 in RBTree<String, Dynamic>::Insert (this=0x7ffff7ee5a44, inKey=..., inValue=...) at /home/maxless/svn/hxcpp/src/hx/RedBlack.h:155
155 root = tmp_head->link[1];

Changing anon object into a full new class is a workaround that I've found.

Now I understand what happens here - each anon object only has a one
RBTree of fields and the implementation is not thread-safe. That code is
probably considered a bad practice but it should work nonetheless. Or at
least it should be documented somewhere.

Reply to this email directly or view it on GitHubhttps://github.com//issues/23
.

from hxcpp.

I have put a fix in RedBlack.h - should fix the problem. Hopefully it is a
net performance gain.

On Tue, Feb 18, 2014 at 10:21 AM, Hugh Sanderson [email protected] wrote:

Yes, generally you are on your own when it comes to locking of structures.
However, I agree you should be able to access (but not create or delete)
items from a hash without a lock. I think the problem here is that the ++
is generating a "set" command and even though the item exists and therefor
no insert will happen, it still iterates over the tree (and mutates it by
the look of the code - I did not write it). I think the best solution
would be for an optimised no-mutate path for the case of when the value is
already in the tree. This should save some allocs too.

On Mon, Feb 17, 2014 at 10:44 PM, maxless [email protected]:

The following example while technically does not have any racing
conditions, produces segfault in HXCPP on Linux 64-bit. It works fine in
Neko environment.

#if neko
import neko.vm.Thread;
#elseif cpp
import cpp.vm.Thread;
#end

class Test
{
var anon: { v1: Int, v2: Int };

public function new()
{
anon = { v1: 0, v2: 0 };
var t1 = Thread.create(t1run);
var t2 = Thread.create(t2run);
}

function t1run()
{
while (true)
anon.v1++;
}

function t2run()
{
while (true)
anon.v2++;
}

public static function main()
{
var t = new Test();
while (true)
Sys.sleep(1);
}
}

Running it through GDB shows that the segfault is in one of the lines in
RBTree implementation (I'm using the latest git version currently, switched
over from using 3.0.2 because of another bug):

0x0000000000477aa6 in RBTree<String, Dynamic>::Insert (this=0x7ffff7ee5a44, inKey=..., inValue=...) at /home/maxless/svn/hxcpp/src/hx/RedBlack.h:159
159 root->red = 0;

0x0000000000477a35 in RBTree<String, Dynamic>::Insert (this=0x7ffff7ee5a44, inKey=..., inValue=...) at /home/maxless/svn/hxcpp/src/hx/RedBlack.h:155
155 root = tmp_head->link[1];

Changing anon object into a full new class is a workaround that I've
found.

Now I understand what happens here - each anon object only has a one
RBTree of fields and the implementation is not thread-safe. That code is
probably considered a bad practice but it should work nonetheless. Or at
least it should be documented somewhere.

Reply to this email directly or view it on GitHubhttps://github.com//issues/23
.

from hxcpp.

I think the best solution would be for an optimised no-mutate path for the case of when the value is already in the tree. This should save some allocs too.

Yes, that sounds perfect.

I have put a fix in RedBlack.h - should fix the problem.

I've tested the changes for some time, it looks stable now both in the test and my application.

Hopefully it is a net performance gain.

I happen to have a rough test available - it creates an anon object, then does some get/sets in a loop, measuring full runtime. With the fixed RedBlack.h anon object set field sped up by ~30% and Reflect.setField() by ~40%. But don't quote me on that, that's just a simplistic measurement for my purposes.

from hxcpp.

BTW, I don't know if it's a known fact or some mistake, but accessing fields of anonymous objects (a = obj.Integer3) is almost twice slower in hxcpp than in neko.

from hxcpp.

Should be good now. Neko converts the string indexes to int hash keys at compile time, and then uses int lookups, so this could be the reason.

from hxcpp.