Apparentlty someone found out it is faster to give data to the OpenGL layer through native bytebuffers, http://www.lwjgl.org/wiki/index.php?title=General_FAQ#Whats_up_with_all_those_ByteBuffers.3F_Why_not_just_use_arrays.3F
Too bad there is no in depth explanation…
<cite>@Momoko_Fan said:</cite> @pspeed: There's also a driver side copy of the texture on the CPU, so deleting it from GL is just as important as deleting from Java's direct memory heap.In any case, here’s what I am proposing:
NativeObject.dispose():
- Reclaims both native buffers and GL memory corresponding to the object.
- Can be called from any thread, since it just en-queues the object for deletion on the next frame by the NativeObjectManager.
- Marks that object as disposed. If assertions are turned ON => an AssertionError is always thrown when trying to use a disposed object. If assertions are turned OFF => it might throw an exception, such as NPE, or Buffer underflow exception, or nothing happens at all. The only guarantee is that there won’t be a native crash.
What do you think?
Some points:
A) The whole proposition comes at the cost that every access to NativeObject now needs an extra check. It may be worth it having a DisposableNativeObject with a dispose() and leave NativeObject without dispose().
B) I think it’s possible to avoid the extra boolean to record the “disposed” state, zeroing out the pointer is enough.
The address to delete is recorded by NativeObjectManager anyway.
C) I’d want to exclude the “nothing happens” case. Code that uses dispose() is fragile, and to get the inevitable bugs diagnosed, stack traces must be generated and made available to the programmer.
Demanding an exception from (Disposable)NativeObject would cover that. It wouldn’t reduce design space for applications, because they can still catch exceptions.
Such a demand wouldn’t incur any runtime overhead because the “disposed” check happens anyway, and the costly exception throw doesn’t happen in the normal, performance-critical case.
D) Do we always want to dispose of CPU buffers and GPU resources at the exact same time?
Some postings sounded as if that’s not always the case, and I could imagine cases for that.
Providing for staged dispose would complicate the API considerably I think. Plus, it can be worked around by splitting the native object in two, one for the CPU buffer and one for the GPU resource; it does come at a presumably small performance penalty.
I don’t have enough information to properly weigh these factors, so I’m leaving the weighing to the JME hive mind 
a Why an extra check, its an assertions, you know how they work or?
c Again you know what assertions are? Because the check is not there always.
d From the user poinst i see no reason to activly delete a object only on one, If your context become invalid (eg resolution changes) a gpu only could not be regenerated, on the other side a cpu only could always be recreated from either the assetmanger, or some kind of user made cache that resides in the heap.
Oh, right, the argument that assertions aren’t enough got deleted during editing, sorry for that.
Use-after-free bugs are nonlocal bugs, i.e. they tend to get overlooked even in rigorous testing by highly trained professionals, so I’d say that the software needs to be protected against these even when assertion checking is turned off.
So all NativeObject descendants should check whether their native pointer is zeroed out before they access it. Risking a JVM crash is a no-no in my book, even if the program passed testing.
For (d), I see the use case of somebody managing GPU resource consumption, e.g. deleting the GPU-side texture but keeping it alive at the CPU side while it’s not being used. Might help for machines that have far less GPU than CPU memory.
That kind of management is already done by opengl internal already, it can freely swap textures around for best vram useage.
I think assertions are enough, no need to waste performance for all users for a feature only very few are using. (And then you can ofcourse keep assertions for that specific class always on if you are paranoid).
I would first go with the most simple approch thats not wasting performance, stuff like gpu and cpu splitting eg can be done later if proven to be really usefull.
No, I’m not paranoid, it’s just common sense:
- use-after-free bugs are the hardest to catch
- software can’t even properly report them (no way to log or report anything after a JVM crash)
That’s why we’re all programming in a language that has automatic garbage collection after all.
Also, I don’t see real overhead concerns ahead:
- as far as I know, the null check is just a single instruction, which shouldn’t contribute much compared to all the other stuff happening with a buffer (there’s a full subroutine call before that after all)
- it can be eliminated completely, by keeping the existing buffer types as they are (without a dispose() function) and creating new DisposableIndexBuffer, DisposableTextureBuffer etc. that have the new dispose() function and do the null checking before every buffer access. Explicitly disposed buffers will be a bit slower as a result, but they’re for cases where memory constraints are tighter than CPU constraints so that shouldn’t matter for @gouessej’s (or anybody else’s) use case.
Anything I’m overlooking here?
(EDIT: I forgot to mention that I’m 100% okay with not complicating the API to support multi-staged dispose.)
@toolforger: There is still potential for native crashes no matter how hard you make your checks, for example:
[java]ByteBuffer data = myImage.getData();
myImage.dispose();
// …
// Sometime later …
// …
data.put(0); // <-- access violation[/java]
Even if you somehow manipulate that object to be invalid (e.g. set capacity and address to zero), you’re still in danger. For example:
[java]ByteBuffer data = myImage.getData().duplicate();
myImage.dispose();
// …
// Sometime later …
// …
data.put(0); // <-- access violation[/java]
Ah right, somehow I was thinking that ByteBuffer is a wrapper around the RAM data, but put() is native.
We’d have to intercept access to disposed native data at a higher level then… which sounds like a lot of work, if it’s possible at all.
This makes me think that having dispose() is probably a bad idea, GC problems notwithstanding.
The contract of dispose() would have to state: Never ever access the native object again. However, how would one check that a given call to dispose() doesn’t violate that? You’d have to be 100% sure that no code ever kept a copy of the native around. The authors of JME probably know where and if that could happen inside JME, but anybody just using JME wouldn’t know. If the object is ever passed to an external library (Nifty, Tonegodgui, Lemur, whatever), even JME experts wouldn’t know off-hand but would have to painstakingly trace all data flows to be 100% sure that no copy is ever retained. Imagine a contribution project that does tricky things with meshes (Blocks libraries etc.); does it keep a copy or not?
I’m feeling very, very uncomfortable around an unchecked dispose().
…or, you could make it really hard for the average user to delete buffers (like now) and recognize that the “experts” will know the risks. For the games where it really matters (like generating geometry) it’s quite clear when buffers can be reclaimed.
For everyone else… use caution. 
I feel fine with unchecked dispose, after all you never HAVE to use it, its only a option that greatly helpes under certains usecases.
Actually its quite simple state in the method that no support whatsoever is given if its used and something wents wrong and be happy with it.
@toolforger but you know that the whole jvm is based around unsafe free malloc ect stuff? It still works quite reliable its just a amtter of being capable to use a clear livecycle of a object. After all all the c++ guys are capable of this as well. (And also there is valgrind wich can be used to find those errors qute effective).
Don’t get me wrong, for me this is not a matter of how hard it is to cache bugs, for me this is a matter of the jvm crashing due to not enough memory since I go on the limits that are possible with current gen graficcards. Of course i could implement tihs myself in my local branch of jme3 without to mcuh hassle, but its a bit stupid that everyone who needs something like this needs to reinvent the wheel again.
<cite>@Empire Phoenix said:</cite> I feel fine with unchecked dispose, after all you never HAVE to use it, its only a option that greatly helpes under certains usecases.Actually its quite simple state in the method that no support whatsoever is given if its used and something wents wrong and be happy with it.
@toolforger but you know that the whole jvm is based around unsafe free malloc ect stuff? It still works quite reliable its just a amtter of being capable to use a clear livecycle of a object. After all all the c++ guys are capable of this as well. (And also there is valgrind wich can be used to find those errors qute effective).
Don’t get me wrong, for me this is not a matter of how hard it is to cache bugs, for me this is a matter of the jvm crashing due to not enough memory since I go on the limits that are possible with current gen graficcards. Of course i could implement tihs myself in my local branch of jme3 without to mcuh hassle, but its a bit stupid that everyone who needs something like this needs to reinvent the wheel again.
So if you use the BufferUtils.destroy on the buffers then you still run out of RAM? Or is the issue that you can’t always get at the buffers?
To me, the issue is the user who in the SDK types .de on and image or something and suddenly the suggestion is to .delete() or .destroy() the object. “Yes, I think I must want to do that.” Then 9 posts into a thread where their JVM is crashing “with the latest update” or some other benign reason then we finally get them to post all of their code (even the stuff that “couldn’t possibly be the reason”) and find they are calling a method that they never ever should have called ever ever ever.
Improper use of this method can crash the JVM or blue screen your OS. It might be better to have to pass the object off to something else non-obvious to get it deleted. I don’t normally recommend package-private methods but this might be the right case for one.
I completely agree with pspeed on this one. Empire, you aren’t thinking with a library developer/maintainer mindset.
There is a fundamental law of computing coming in here. If as the developer of a library you add a button that says “press this to destroy the world”. You then have a safety cover over it, a post-it note saying NEVER PRESS THIS, and a padlock on the safety cover
- Someone WILL press the button
- They will then blame you when the world is destroyed
- They will forget they actually pressed the button and claim no-one would ever press that button until 10 times around they finally admit well maybe they did press the button, but surely that wasn’t relevant.
<cite>@Empire Phoenix said:</cite>@toolforger but you know that the whole jvm is based around unsafe free malloc ect stuff? It still works quite reliable its just a amtter of being capable to use a clear livecycle of a object. After all all the c++ guys are capable of this as well. (And also there is valgrind wich can be used to find those errors qute effective).
The JVM people have been spending person-decades on getting all the memory problems out, yet even today I’m seeing JVM crashes while debugging. With 100% pure Java programs, mind you (but with debugging features like hot code replacement in - obviously, not all bugs have been shaken out from those code paths).
Still I get the occasional JVM crash when debugging. So even man-years of top-notch engineering doesn’t fully protect against that. Now go and tell me that all JME users are top-notch and I know you’re delusional 
The C++ guys don’t manage to get that right either. Just look at the multitude of CVEs that keep flowing in; the bug reports keep flowing for Javascript, Mozilla, you-name-it. Flash crashes on a regular basis. Etc. etc. pp.
Valgrind helps, but it is definitely not catching everything. The only thing that can catch all bugs is checking every single access. If that’s too costly, you can try optimizing the hell out, e.g. providing a foreach loop and checking only lower and upper bounds, and similar things; you still have to check the optimizer, but at least it’s just a comparatively small piece of library software that you have to check and not the whole world of application software that might have bugs.
Also, what pspeed and zarch said.
Expounding on zarch’s example, here’s point 4:
4. Somebody WILL build a nice-looking package around the button, with padlock, post-it, and safety cover removed and placing his own safety package around it. If you tumble the package in the wrong direction, some internal machinery will unhinge, tear through the builtin safety and press the button anyway.
Translated to real-world terms, third-party modules will require close monitoring whether they dispose() or not. And whether they’re really safe in their usage of dispose(). And we WILL have situations where a bug slipped through all monitoring.
JME is built for rock solidity even in the face of misuse and neglect by application programmers, and it works; in the time I’ve been watching the forums, there was not a single call for help where a problem would mysteriously appear or disappear depending on call order. It’s one of the hallmarks of coding with Java that while you can do all kinds of wrong, the root cause of any bug is tracable by looking at most at limited number of call chains (O(log(N)) for a program of size N), not by looking at the entire program (an O(N) task, and since the number of bugs grows roughly linearly with the program size, developing a program of size N amounts to O(N^2)).
Well its not that hard to protect against,
Make destroy deprecated to start with, add awarning and a big fat error logging when it is actually used.
The bytebuffers itself are a neglibable problem, as i can kill them myself without to much hassel already, but the nativeobjects are currently impossible to delete without chaning part of the core.
Also where possible assertions can be used to catch that kind of errors, so you get more or less both, stability and performance.
So how about an alternative for that?
Deprecation warnings don’t work, they carry the wrong connotations and tend to be misinterpreted by tools.
(Nifty is currently recovering from exactly that mistake.)
I’d avoid logging for mostly the same reasons. Well, it could be at warning level; error is too strong.
Adding ample warnings to Javadoc and assertions to the code would be a good start.
Whatever we do, the best that can be achieved is to mitigate the risk that dispose() will be to JME’s stability.
I think it’s a judgement call whether the residual risk is worth the gain; by nature, this will be judged differently by those who can make use of the gain and those who cannot.
1 Like
Why not name it disposeAndRiskCrash() ^^
LOOOL… it’s better than deprecation and more effective than deprecation, so if some dispose() routine is the way to go, this has my vote 
<cite>@toolforger said:</cite> Deprecation warnings don't work, they carry the wrong connotations and tend to be misinterpreted by tools. (Nifty is currently recovering from exactly that mistake.)I’d avoid logging for mostly the same reasons. Well, it could be at warning level; error is too strong.
Adding ample warnings to Javadoc and assertions to the code would be a good start.
Whatever we do, the best that can be achieved is to mitigate the risk that dispose() will be to JME’s stability.
I think it’s a judgement call whether the residual risk is worth the gain; by nature, this will be judged differently by those who can make use of the gain and those who cannot.
The “best we can do” is make this method uncallable directly… make it package-private or protected and require passing the object to something else (the context, the renderer, the NativeObjectManager, etc.) to actually do the destroy.
I’d certainly love to see that happen, but is it possible?
Some Mesh code (including code in contribution) does direct buffer manipulation - index buffers and vertex buffers, mostly.
Hand a disposed buffer to them and they’ll either crash or incur the overhead of null checking on every friggin’ put() call.
OTOH most of that code uses the auto-incrementing put() and doesn’t seem to care much about the performance baggage involved in that… maybe I’m worrying too much and a null check would be negligible.
I guess that should be benchmarked. I.e. get some put()-heavy code, add null checks to the put() functions, and see if there’s any visible impact on the FPS display on a slightly older Android. (I’d do the benchmark myself but I guess results from a desktop application wouldn’t tell us much.)
<cite>@toolforger said:</cite> I'd certainly love to see that happen, but is it possible? Some Mesh code (including code in contribution) does direct buffer manipulation - index buffers and vertex buffers, mostly. Hand a disposed buffer to them and they'll either crash or incur the overhead of null checking on every friggin' put() call.OTOH most of that code uses the auto-incrementing put() and doesn’t seem to care much about the performance baggage involved in that… maybe I’m worrying too much and a null check would be negligible.
I guess that should be benchmarked. I.e. get some put()-heavy code, add null checks to the put() functions, and see if there’s any visible impact on the FPS display on a slightly older Android. (I’d do the benchmark myself but I guess results from a desktop application wouldn’t tell us much.)
I’m not trying to protect against the crashes. I’m trying to protect noobs from calling the method by accident. And in this case, to me “noob” is anyone who has never programmed seriously in C or C++.
The fact that the “method that should never be called” will pop right up in the method suggestions in the IDE bugs the hell out of me. “Derrrr… yes, I think I DO want to delete my image now so that the memories are more better freely emptier…”