acquireTransientContext does heap allocation

[sorum40]

Hi;

I saw that acquireTransientContext does heap allocation using new.
While debugging, I saw this for most if not all draw calls (clear, draw etc...)

Is it possible to have it not use heap allocations everytime (like once at the start of the app, or use a class member)?

Many thanks;

[eXpl0it3r]

What's the issue?

[sorum40]

What's the issue?

Performance reasons.
With many draw calls, doing heap allocations is an overhead as it translate in OS API call.

I don't know if the allocations are done per texture switch or not.

But it would be great if it is possible to eliminate all the heap allocations altogether.

That is after all the resources are loaded, no more heap allocations anywhere.

Is it possible to have acquireTransientContext work without the "new" and "delete" calls?

Thank you;

[Laurent]

What "new" are you talking about? The only one I can see when I look at that function, is called only once per thread.

[sorum40]

What "new" are you talking about? The only one I can see when I look at that function, is called only once per thread.

Thanks for the reply.

I don't see it once per thread, I see it for maybe all draw and clear.

void GlContext::acquireTransientContext()
{
    // Protect from concurrent access
    Lock lock(mutex);

    // If this is the first TransientContextLock on this thread
    // construct the state object
    if (!transientContext)
        transientContext = new TransientContext;

    // Increase the reference count
    transientContext->referenceCount++;
}

On the same frame, everytime there is a draw or clear, it hits acquire, hits new. And release again later.

I have only 2 threads, one loading and one main.
The loading doesnt' do any draw, and go to sleep after loading.

The main thread does draw and clear, and it's the one hitting the "new" in acquire all the time.

I catch it using the _CrtSetAllocHook function.

[eXpl0it3r]

Performance reasons.
With many draw calls, doing heap allocations is an overhead as it translate in OS API call.

Since you talk about performance, I assume you ran a profiler. What kind of bottleneck did the profiler report?

transientContext exists only once per thread and as you can see in the code you post, it only get allocated if it wasn't allocated already.

    // This per-thread variable tracks if and how a transient
    // context is currently being used on the current thread
    sf::ThreadLocalPtr<TransientContext> transientContext(NULL);

[sorum40]

With many draw calls, doing heap allocations is an overhead as it translate in OS API call.
Since you talk about performance, I assume you ran a profiler. What kind of bottleneck did the profiler report?

Yes, run profiler on other situation involving heap allocations, not this.
I prefer no heap allocations anywhere after loading resources.

transientContext exists only once per thread and as you can see in the code you post, it only get allocated if it wasn't allocated already.

    // This per-thread variable tracks if and how a transient
    // context is currently being used on the current thread
    sf::ThreadLocalPtr<TransientContext> transientContext(NULL);

Then why I get alloc/dealloc all the time in the same frame, and the same main thread?
Continously calling new and delete on the transientContext.

This is a snapshot in the same frame, the same thread:

Not Flagged   >   6188   0   Main Thread   Main Thread   Game.exe!MemoryAllocHook   Normal
                        Game.exe!MemoryAllocHook(int nAllocType, void * pvData, unsigned int nSize, int nBlockUse, long lRequest, const unsigned char * szFileName, int nLine) Line 54   
                        [External Code]   
                        [Frames below may be incorrect and/or missing, no symbols loaded for ucrtbased.dll]   
                        Game.exe!sf::priv::GlContext::acquireTransientContext() Line 316   
                        Game.exe!sf::GlResource::TransientContextLock::TransientContextLock() Line 63   
                        Game.exe!sf::Texture::bind(const sf::Texture * texture, sf::Texture::CoordinateType coordinateType) Line 619   
                        Game.exe!sf::RenderTarget::applyTexture(const sf::Texture * texture) Line 492   
                        Game.exe!sf::RenderTarget::clear(const sf::Color & color) Line 106   
                        Game.exe!PD_Screen::Clear() Line 53   
                        Game.exe!GameManager::Draw() Line 501   
                        Game.exe!GameManager::Run() Line 337   
                        Game.exe!Run() Line 44   
                        Game.exe!main() Line 55   
                        [External Code]   


                  
                  Not Flagged   >   6188   0   Main Thread   Main Thread   Game.exe!MemoryAllocHook   Normal
                        Game.exe!MemoryAllocHook(int nAllocType, void * pvData, unsigned int nSize, int nBlockUse, long lRequest, const unsigned char * szFileName, int nLine) Line 54   
                        [External Code]   
                        [Frames below may be incorrect and/or missing, no symbols loaded for ucrtbased.dll]   
                        Game.exe!sf::priv::GlContext::releaseTransientContext() Line 339   
                        Game.exe!sf::GlResource::TransientContextLock::~TransientContextLock() Line 70   
                        Game.exe!sf::Texture::bind(const sf::Texture * texture, sf::Texture::CoordinateType coordinateType) Line 669   
                        Game.exe!sf::RenderTarget::applyTexture(const sf::Texture * texture) Line 492   
                        Game.exe!sf::RenderTarget::clear(const sf::Color & color) Line 106   
                        Game.exe!PD_Screen::Clear() Line 53   
                        Game.exe!GameManager::Draw() Line 501   
                        Game.exe!GameManager::Run() Line 337   
                        Game.exe!Run() Line 44   
                        Game.exe!main() Line 55   
                        [External Code]   

Not Flagged   >   6188   0   Main Thread   Main Thread   Game.exe!MemoryAllocHook   Normal
                        Game.exe!MemoryAllocHook(int nAllocType, void * pvData, unsigned int nSize, int nBlockUse, long lRequest, const unsigned char * szFileName, int nLine) Line 54   
                        [External Code]   
                        [Frames below may be incorrect and/or missing, no symbols loaded for ucrtbased.dll]   
                        Game.exe!sf::priv::GlContext::acquireTransientContext() Line 316   
                        Game.exe!sf::GlResource::TransientContextLock::TransientContextLock() Line 63   
                        Game.exe!sf::Texture::bind(const sf::Texture * texture, sf::Texture::CoordinateType coordinateType) Line 619   
                        Game.exe!sf::RenderTarget::applyTexture(const sf::Texture * texture) Line 492   
                        Game.exe!sf::RenderTarget::draw(const sf::Vertex * vertices, unsigned int vertexCount, sf::PrimitiveType type, const sf::RenderStates & states) Line 259   
                        Game.exe!sf::Sprite::draw(sf::RenderTarget & target, sf::RenderStates states) Line 145   
                        Game.exe!sf::RenderTarget::draw(const sf::Drawable & drawable, const sf::RenderStates & states) Line 195   
                        Game.exe!PD_Screen::Draw(const ISprite & xSprite, RenderState::BLEND_MODE eBlendMode, const IShader * pxShader) Line 61   
                        Game.exe!Widget::Draw(IRenderTarget & xRenderTarget, RenderState::BLEND_MODE eBlendMode) Line 137   
                        Game.exe!Gui::Draw(IRenderTarget & xRenderTarget) Line 64   
                        Game.exe!MenuHandler::Draw(IRenderTarget & xRenderTarget) Line 46   
                        Game.exe!GameManager::Draw() Line 508   
                        Game.exe!GameManager::Run() Line 337   
                        Game.exe!Run() Line 44   
                        Game.exe!main() Line 55   
                        [External Code]   
                  
                  
                  
                  Not Flagged   >   6188   0   Main Thread   Main Thread   Game.exe!MemoryAllocHook   Normal
                        Game.exe!MemoryAllocHook(int nAllocType, void * pvData, unsigned int nSize, int nBlockUse, long lRequest, const unsigned char * szFileName, int nLine) Line 54   
                        [External Code]   
                        [Frames below may be incorrect and/or missing, no symbols loaded for ucrtbased.dll]   
                        Game.exe!sf::priv::GlContext::releaseTransientContext() Line 339   
                        Game.exe!sf::GlResource::TransientContextLock::~TransientContextLock() Line 70   
                        Game.exe!sf::Texture::bind(const sf::Texture * texture, sf::Texture::CoordinateType coordinateType) Line 669   
                        Game.exe!sf::RenderTarget::applyTexture(const sf::Texture * texture) Line 492   
                        Game.exe!sf::RenderTarget::draw(const sf::Vertex * vertices, unsigned int vertexCount, sf::PrimitiveType type, const sf::RenderStates & states) Line 259   
                        Game.exe!sf::Sprite::draw(sf::RenderTarget & target, sf::RenderStates states) Line 145   
                        Game.exe!sf::RenderTarget::draw(const sf::Drawable & drawable, const sf::RenderStates & states) Line 195   
                        Game.exe!PD_Screen::Draw(const ISprite & xSprite, RenderState::BLEND_MODE eBlendMode, const IShader * pxShader) Line 61   
                        Game.exe!Widget::Draw(IRenderTarget & xRenderTarget, RenderState::BLEND_MODE eBlendMode) Line 137   
                        Game.exe!Gui::Draw(IRenderTarget & xRenderTarget) Line 64   
                        Game.exe!MenuHandler::Draw(IRenderTarget & xRenderTarget) Line 46   
                        Game.exe!GameManager::Draw() Line 508   
                        Game.exe!GameManager::Run() Line 337   
                        Game.exe!Run() Line 44   
                        Game.exe!main() Line 55   
                        [External Code]   



MemoryAllocHook detects heap allocations/deallocations.
It looks like the transientContext is newed, then deleted in releaseTransientContext, then newed again, then released etc...

So as you can see, it gets allocated all the time, as it is getting deleted all the time.

If this is the normal behaviour, is it possible to make it in a way to avoid new and delete?

[eXpl0it3r]

Yes, run profiler on other situation involving heap allocations, not this.
I prefer no heap allocations anywhere after loading resources.

This is the real issue here. There's no point in trying to "optimize" something that's not an issue in the first place.
So once up on a time you ran a profiler on some other code that allocated memory and what was the conclusion there? Can the same conclusions be applied to this situation?

Your assumption of "heap allocations == bad/evil" is just wrong and your testing with MemoryAllocHook is just feeding you a false view. If you're using Visual Studio, use the built in Memory Profiler and you'll see that there are tens of thousands of allocations happening in GDI and other places.
Thinking that one little allocation has to be removed for unprofiled performance improvement is just ridiculous.

If you're making a game (or an app), then there's so much more important things you should be investing your time in, than hunting down heap allocations.

Here is a short list of questions that in my opinion are more important and this is really just a short list!
How's the art going? Are the game mechanics made as fun as humanly possible? How is the user experience and usability of your UI - anything to improve on? Can you made add more than one hero? Is your rendering really smooth? Did you account for proper interpolation? Or would Box2D offer a better solution over all and how would you integrate it? How is the audio design coming along? Did you consider how you can make use of 3D spatialization? How are you packing your resources? Maybe consider using PhysicsFS or write your own custom format? What about parsing settings and custom animation files? Did you consider already how to distribute it? What platforms are you targeting? Do you have access to all of them? Do you know what dependencies need to additionally be built and included? How does macOS work again with that ResourcePath? Or how do you deal with custom library versions on Linux - can you package them with the game? An installer for Windows or just a zip file?

So as you can see, it gets allocated all the time, as it is getting deleted all the time.

If this is the normal behaviour, is it possible to make it in a way to avoid new and delete?

I talked to binary1248 again and while it's thread local, the transient context lock is required inside bind and there's no way around it.

[sorum40]

So there we have confirmation that the transientContext being created once per thread is wrong, isn't it?
It is created and recreated endlessly.

A documentation update will be good, to clear the confusion.

If there is no way around it, and this seems to be the case, I can live with it, no problem.

[binary1248]

So there we have confirmation that the transientContext being created once per thread is wrong, isn't it?

It is working as designed... which is far from wrong. In order to track context usage on a thread-level we need to use thread local storage, which as many should know, forces us to resort to dynamic memory allocation pre-C++11. If you didn't stop skimming through the code as soon as you saw the new operator, you would have seen that allocating a TransientContext object doesn't even cause new contexts to be created if one is already active on the current thread. As such, besides the fact that TransientContext is allocated dynamically there is nothing else to complain about performance-wise.

It is created and recreated endlessly.

Just like the vertex memory your graphics driver allocates, the SFML window events, audio buffers, Win32 events and the other 99% of the stuff happening on your system when you draw a frame? Yes. Seriously... in the grand scheme of things this makes little to no difference. If you choose to look at 1% of the total allocations per frame and behave like they pose the biggest problem in your application you need to brush up on your performance profiling skill set a bit.

Suppose we did change the code to reduce the allocations, how could we tell if it made the state of the library better or worse? We don't know how the runtime manages memory. Maybe it uses a memory pool or arenas internally. It might do it better than we would because it has access to more information than we do. Re-inventing wheels that we assume aren't there just because we don't have access to the underlying source code is never a good idea.

SFML is an open source library, and like all software that is developed and maintained, it takes effort to do so. In order to make the most out of this effort, we look for things that we can change that will have a noticeable impact on some aspect of the library. There is always space for micro-optimizations but not when the list of important things to do is already as long as it currently is.

A documentation update will be good, to clear the confusion.

Like GlResource and many other things in the library, this is an internal implementation detail and should not be documented as part of the public API. If you really expect us to document every spot where dynamic memory is allocated and freed, maybe you should consider starting with the C/C++ standard library first.

[sorum40]

It's already a good thing that I didn't see any other SFML allocations for many frames.
The only one that shows up from SFML, is also the only the only that shows UP while running the game, and it is this transientContext.

As I said, not a big issue, and I a can live with it.

If it is changed in a future update to not do dynamic allocations multiple times that would be good.

In any case, thank you all for this great library.