Open Bug 833185 Opened 7 years ago Updated 7 years ago

graphics memory reporters are not good


(Core :: Graphics, defect)

Not set




(Reporter: bjacob, Unassigned)



Noticed this while reviewing bug 831193; see bug 831193 comment 34.

Problem 1: I thought that about:memory's approach to memory accounting was explicitly to re-count everything everytime, and not to use accumulating counters, as explained
But the counters here are violating this good principle.

Problem 2: sAmount is static, and OpenGL work is done on more than one thread (for now on Android, in the near future on other platforms). So shouldn't these operations be made atomic? Or is there a guarantee that only the OpenGL memory of one of the threads will ever be reported in this way? If so, is that an acceptable limitation?

Taking a step back, is this really the right approach to accounting texture memory? Why do it in such a special way, instead of instrumenting directly at the level of GL entry points in GLContext, where we already have the MOZ_GL_DEBUG implementation, the framebuffer-0-spoofing, etc? That would be a lot more fool-proof. It doesn't count exactly the same thing, but I'd like to hear a rationale for why you think that the thing measured here is more relevant, and will remain so, as it looks extremely bitrot-prone (because of it being so special) whereas general GL memory accounting would not bitrot so easily (because it would be agnostic wrt how we use the GL). Jeff Gilbert's patches in bug 811181 (why haven't they landed yet?) lay the right foundation for implementing this properly.
Traversal-based reporters are definitely better.  They can't always be used, but if they can in these cases, we should convert them.

sAmount probably should be modified atomically, yes.

I don't know about the more general question of the right approach.  Making things more foolproof sounds good, though!
I also have no objections to doing this in some other, better way. I was under the impression that we couldn't count the memory in GL buffers which is why we were using the accumulating counter.
We can't query the size of these buffers, but we can *probably* estimate pretty well. There's no way to know for sure, though.
You need to log in before you can comment on or make changes to this bug.