next »

[LeoMaheo]

Hi developers,

I'm contemplating using org.openmali.vecmath2 as math library for those (future) end users that write artificial intelligence (AI) logic for a game of mine.

However, I cannot use vecmath2 for this right now, because of those fromPool and toPool methods: I cannot expose non-thread safe functionality to end users, since their AI:s run simultaneously in separate threads.

A configuration option could be added, that replaces those static pool fields with thread local storage (and thus renders OpenMaLi thread safe (in combination with some other minor changes)).

(The config option could be a simple function call, done at startup by applications that need a completely thread safe OpenMaLi. Probably only the very first function invokation should be considered, so the threading policy cannot be changed later.)

Would this be reasonable?
(  = If I write a sensible patch, would you apply it?)

Regards, Magnus

[Marvin Fröhlich]

These pooling methods ARE thread safe. Actually they are the most thread safe part of vecmath2 . The rest of vecmath2 is not thread safe for invoking-different-operations-on-the-same-object, but are thread safe for invoking-the-same-method-on-different-objects. The ObjectPool's methods are all synchronized. So they are really threas safe.

If there is a bug, I would happily accept your patch. If you want, I can even grant you dev-access on OpenMaLi, so that you can apply the patch by yourself and possibly add extensions or other patches. Of course a short note here on the DEv-forum would be nice before you do anything.

Marvin

[LeoMaheo]

The ObjectPool's methods are all synchronized. So they are really threas safe.

Oh I only checked the Vector3fPool and other concrete subclasses; didn't notice those synchronized methods in ObjectPool, sorry.

If there is a bug,

Hmm, here are some usages of static mutable variables (but I have written no patch):

org.openmali.spatial.bodies.Plane: Vector3f temp,
org.openmali.vecmath2.Vector4f: Vector4f ZERO
org.openmali.vecmath2.TupleNf: StringBuffer TMP_SB
org.openmali.vecmath2.doubles.TupleNd: StringBuffer TMP_SB

(Used this Bash command to find those:
find -type f | xargs grep 'static' | grep -v '\.svn' | grep '=' | egrep -v 'long|float|int|boolean|double'  )

If you want, I can even grant you dev-access on OpenMaLi, so that you can apply the patch by yourself and possibly add extensions or other patches.

Oh, thanks. However, no time for that now --- I need to investigate PhysX, and, actually, I'm contemplating moving from jME to Xith3D because of Xith3D's built in model loading support. But so much work involved, don't know what I'll do  (I was just asking about OpenMaLi now, to find out what might be possible or not, in the future.)

Thanks,
Magnus

[Marvin Fröhlich]

org.openmali.spatial.bodies.Plane: Vector3f temp,

Yes. I will fix that.

org.openmali.vecmath2.Vector4f: Vector4f ZERO

This is a read-only constant. So there's no problem.

org.openmali.vecmath2.TupleNf: StringBuffer TMP_SB
org.openmali.vecmath2.doubles.TupleNd: StringBuffer TMP_SB

This is only used for the toString() method. So I don't expect this to be a problem. But I can fix it anyway.

Oh, thanks. However, no time for that now --- I need to investigate PhysX, and, actually, I'm contemplating moving from jME to Xith3D because of Xith3D's built in model loading support. But so much work involved, don't know what I'll do  (I was just asking about OpenMaLi now, to find out what might be possible or not, in the future.)

When you have investigated PhysX, you may want to write an XPAL implementation for it, so that it can be used through Xith3D's physics abstraction. This would be an awesome contribution. But forget about it, if you don't have time for it.

Marvin

[LeoMaheo]

org.openmali.vecmath2.Vector4f: Vector4f ZERO

This is a read-only constant. So there's no problem.

But usage of the read-only constructor has been forgotten:

Code:

public static final Vector4f ZERO = new Vector4f( 0f, 0f, 0f, 0f ); // should be: Vector4f(true, ....)

(By the way, I'll probably not expose OpenMaLi to end users of my game (which will probably never be finished), because it must not happen that all end users are forced to update their code because of changes made to OpenMaLi. I believe I need a maths API that is almost carved in stone, or one that I am in total control over. Perhaps I'm a bit paranoid/pedantic, hmm. Actually, were I to expose OpenMaLi to my end users, they could destroy each other's artificial intelligence modules (which execute simultaneously in separate threads) by maliciously keeping references to pooled vectors, and mutating those vectors even after they've been returned to the pool.
  --- Anyway, I'll be happy to use OpenMaLi in my core application if I switch to Xith   )

When you have investigated PhysX, you may want to write an XPAL implementation for it, so that it can be used through Xith3D's physics abstraction. This would be an awesome contribution. But forget about it, if you don't have time for it.

I'll keep that in mind

Magnus

[Marvin Fröhlich]

But usage of the read-only constructor has been forgotten:

Code:

public static final Vector4f ZERO = new Vector4f( 0f, 0f, 0f, 0f ); // should be: Vector4f(true, ....)

Ah, ok. This is true. I will fix that. Thanks a lot.

(By the way, I'll probably not expose OpenMaLi to end users of my game (which will probably never be finished), because it must not happen that all end users are forced to update their code because of changes made to OpenMaLi. I believe I need a maths API that is almost carved in stone, or one that I am in total control over. Perhaps I'm a bit paranoid/pedantic, hmm. Actually, were I to expose OpenMaLi to my end users, they could destroy each other's artificial intelligence modules (which execute simultaneously in separate threads) by maliciously keeping references to pooled vectors, and mutating those vectors even after they've been returned to the pool.
  --- Anyway, I'll be happy to use OpenMaLi in my core application if I switch to Xith   )

It would be possible to protect an instance for write access when it was pushed back to the pool. Would that help you?

Marvin

[LeoMaheo]

Actually, were I to expose OpenMaLi to my end users, they could destroy each other's artificial intelligence modules (which execute simultaneously in separate threads) by maliciously keeping references to pooled vectors, and mutating those vectors even after they've been returned to the pool.

It would be possible to protect an instance for write access when it was pushed back to the pool. Would that help you?

Tagging it read-only when put back into the pool? But a malicious AI thread could keep a reference to it, modify it all the time from within a loop, catching (ignoring) exceptions. Eventually, some other AI thread will retrieve the instance from the pool, read-only-flag removed and ... the attacking AI can modify it to destroy the other AI's computations.
  ( Perhaps per-thread storage is actually the only way to thwart such an attack? I wonder if accessing per-thread storage is more or less expensive than entering a synchronized block. But I'm not suggesting per-thread stacks now --- it might as well happen that I use e.g. the original vecmath lib instead, solely because it's carved in stone. Or I might use no math lib at all, just provide a Vec3f and a Quat4f and nothing more, letting end users choose math lib themselves --- and then they can use OpenMaLi, since I use separate class loaders for separate AI's, so they cannot mess up each other's static variables (but the core math lib is loaded by the shared application class loader. ))

But from a debugging point of view tagging pooled instances as read-only could perhaps be useful.

[Marvin Fröhlich]

Well, I did't think about it enough. The tagging readonly doesn't help here. Though you can always create your own Vector3fPool, Point3fPool, etc. so that they don't interfere.

On the other hand instances from a pool should only be used as method-local variables. Where else should they be useful? So this should never be a problem actually.

The vecmath2 part of OpenMaLi is more or less carved in stone. There will only be additions, but no changes in the API, that will force you to change your application code. So I don't see a problem and you could even expose vecmath2 to your users.

Marvin

[LeoMaheo]

The tagging readonly doesn't help here. Though you can always create your own Vector3fPool, Point3fPool, etc. so that they don't interfere.

( Create own? Hmm, I would probably have to modify OpenMaLi (or my local copy of it) to convert those thread pools to per thread pools, if that's what you meant. )

On the other hand instances from a pool should only be used as method-local variables. Where else should they be useful? So this should never be a problem actually.

But people might deliberately make it a problem. Actually, I believe some of my end users would do so (ignoring for a second that my game will never be finished), keeping in mind that cheating in online games is fairly common. In my case the effects would be fairly funny I think: maths computation being destroyed, causing... no idea --- creatures running away randomly, or stumbling and falling. Not only cheating, but even destroying for other AI:s, that must be... tantalizing ?

The vecmath2 part of OpenMaLi is more or less carved in stone. There will only be additions, but no changes in the API, that will force you to change your application code. So I don't see a problem and you could even expose vecmath2 to your users.

That's interesting. If the API is reasonably stable, and if I switch to Xith, then it seems a reasonable/good approach for me to expose vecmath2 and eventually fix that cheating-pooling issue. (Right now I convert vectors & quaternions from/to Sun's vecmath, cumbersome.)

Thanks,
Magnus

[Marvin Fröhlich]

( Create own? Hmm, I would probably have to modify OpenMaLi (or my local copy of it) to convert those thread pools to per thread pools, if that's what you meant. )

No, that's not, what I meant. See the following code:

Code:

// Store this pool instance somewhere accessible for the authorized audience.
Vector3fPool pool = new Vector3fPool( 16 );

...

// To use a polled instance:

Vector3f v = pool.alloc();

v.doSomethingWithMe();

pool.free( v );

Marvin

[Marvin Fröhlich]

I have fixed the bugs above in OpenMaLi.

Marvin

[LeoMaheo]

( Create own? Hmm, I would probably have to modify OpenMaLi (or my local copy of it) to convert those thread pools to per thread pools, if that's what you meant. )

No, that's not, what I meant. See the following code:

Ok, I was stubbornly thinking about creating classes, not instances.

***

( In case it matters, I currently have no plans to switch to OpenMaLi --- so much work involved, and after thinking for a week I think Sun's Vecmath works reasonably fine for me.
Anyhow, some performance notes and other clarifications below. )

The below snippet should be what's needed to make Vector3f cheat safe:

Code:

  private static final Vector3fPool POOL =
    @Override protected Vector3fPool initialValue() {
      new ThreadLocal<Vector3fPool>() {
        return new Vector3fPool( 128 );
      }
    };

  public static Vector3f fromPool()
  {
    return POOL.get().alloc();
  }
 
  // other fromPool(...) modified in the same manner

  public static void toPool( Vector3f o )
  {
    POOL.get().free( o );
  }

As far as I've seen, this would improve performance --- the pool classes need no longer be synchronized. If separate processor cores make heavy usage of OpenMaLi, thread local pools performs better than synchronized static instances: The synchronization would cause contention.

On the other hand, in a single threaded environment, there might not be much of a difference. I've done some measurements, and it seems  the ThreadLocal version is mariginally faster than synchronized static Pool instances. However, someone else run the same benchmark, and it seems on his computer ThreadLocal was significantly faster (guess it depends on lots of stuff, e.g. jvm version, HotSpot server or client, OS).

Here is a thread about my performance measurements. (I was very surprised that allocating *new* memory was sometimes much faster than even using an unsynchronized stack instance! Object recycling, someone said. However, the Vec3 I used in the test is more light-weight than vecmath2.Vector3f.)

http://forums.sun.com/thread.jspa?threadID=5331680

***

I have not properly described the game I'm constructing, so I'm not sure you know what I mean when I mention cheating every now and then. I'll try to explain.

The game loads different AI modules that compete with each other. They execute in parallel, one thread per AI. If one AI module named "Evil" can access data used by another AI module, then "Evil" can corrupt that data for the other AI module. Therefore, AI modules are loaded by separate class loaders. (They execute with no permissions.)

However, the maths library is shared by the game core and all AI modules (it's loaded by the application class loader), so if there is any mutable static data in there, "Evil" can modify it, and if some other AI module uses that static mutable object, then results would be undefined. So, since those vecmath2 pools are static and mutable, the AI modules would not be able to use them (without risking an attack from "Evil").

So that's why I wouldn't want to expose the static pools: because my end users would use them (I'd expect them to ignore any warnings), and my game would be prone to cheating.

***

To summarize,

It seems that unsynchronized pools, with thread local "static" instances, that's the way to go with regard to performance, and to ensure OpenMaLi is not prone to cheating when exposing it to e.g. competing AI modules running in parallel.


Thanks,
Magnus

[Marvin Fröhlich]

It's your decision. And I won't tell you to do anything else. I just don't understand it. Your suggestion about ThreadLocal usage is definitely a good idea, because it not only solves the cheating problems, but is also no slower or even faster. I will take it into OpenMaLi. And I think, I will also simply move the contents of the doubles package into the vecmath2 package.

You will have zero influence to Sun's vecmath lib. And you won't have out-of-the-box pools in Sun's vecmath at all. OpenMaLi has many more features, that you don't have in Sun's vecmath, such as read-only instances, static final instances like Colorf.RED, etc. and many others. If there's anything, that you would like to be in OpenMaLi, it can easily be added. There's absolutely no chance to do this with Sun's vecmath. All in all, there's no good reason to use Sun's vecmath. But it's your decision .

Marvin

[LeoMaheo]

I'll clarify how I think.

But that'll be another thread
  -- no longer related to threading issues :-)

Here:

[Marvin Fröhlich]

I have moved the double-implementations into the vecmath2 package and implemented the ThreadLocal support. Please check, if it is as you wanted it.

Marvin