engineFreeModel has delay if model is created

[TheNormalnij]

Describe the bug engineFreeModel doesn't free model in same frame.

To reproduce Run this code:

local IDs = {}

localPlayer:setPosition( 0, 1 , 5 )

for i = 1, 3 do
    local id = engineRequestModel( 'ped' )
    outputChatBox( id )
    IDs[i] = id
    Ped( id, 0, 0, 5 + i )
end

for i = 1, 3 do
    engineFreeModel( IDs[i] )
end

for i = 1, 3 do
    local id = engineRequestModel( 'ped' )
    outputChatBox( id )
    IDs[i] = id
end

outputChatBox should return same values

Expected behaviour engineFreeModel should free model after calling.

Version After this fix #1846

[Pirulax]

I dont think this would have a usecase. Who requests and then immediately frees a model?

[TheNormalnij]

I have world loader. I use almost all free id's. This bug can produce case, when prev world not fully unloaded and i can't load next world because i don't have free ID's.

[TheNormalnij]

1842 fixes same bug without this issue

[qaisjp]

I think this is known and deliberately done this way.

If I recall correctly, engineFreeModel marks a model number as unused, and doesn't immediately free it, and only is only truly freed when a (future) call to engineRequestModel needs to use that model number.

What do you mean by the "lag" in the issue title? No such lag is mentioned in the issue description

outputChatBox should return same values

This is incorrect. The wiki deliberately documents that models are requested in a non-deterministic manner.

[TheNormalnij]

I think this is known and deliberately done this way.

It's new behavior. You think about m_Models this array uses this way.

If I recall correctly, engineFreeModel marks a model number as unused, and doesn't immediately free it, and only is only truly freed when a (future) call to engineRequestModel needs to use that model number.

Yeah, but now engineRequestModel don't use marked as free models in same frame. You need wait next frame to use freed models. It's lag.

The wiki deliberately documents that models are requested in a non-deterministic manner.

It's almost true. We can't guarantee same order. But MTA client search for first free ID. So if i request { 0, 1, 2, 3 } and free { 0, 1, 2, 3 } engineRequestModel should return { 0, 1, 2, 3 } in same frame but now it's { 4, 5, 6, 7 } and { 0, 1, 2, 3 } in next frame. Old behavior works good in this case.

[qaisjp]

Yeah, but now engineRequestModel don't use marked as free models in same frame. You need wait next frame to use freed models. It's lag.

Ahh, okay 👍

[ghost]

I think this is known and deliberately done this way.

@qaisjp Yes, correct.

It's new behavior.

I know, and it's the right way to do it. engineFreeModel doesn't delete the model interface and this causes a memory leak. But now if I hadn't changed the behavior and fixed the memory leak, the game would crash because there would be dangling pointers. And I don't think engineFreeModel or engineRequestModel causes lag. You'll either have to explain it or support your argument with benchmarks.

It's almost true. We can't guarantee same order. But MTA client search for first free ID. So if i request { 0, 1, 2, 3 } and free { 0, 1, 2, 3 } engineRequestModel should return { 0, 1, 2, 3 } in same frame but now it's { 4, 5, 6, 7 } and { 0, 1, 2, 3 } in next frame. Old behavior works good in this case.

It was already documented on the wiki that the order can change. I don't see why it's a problem.

When you free the model using engineFreeModel, then you can call engineRequestModel in onClientPreRender until a model has been allocated. For example, this code does not cause any lag after you free a model:

local allocatedId = nil
addEventHandler("onClientPreRender", resourceRoot,
    function()
        if not allocatedId then return end
        allocatedId = engineRequestModel("ped")
    end
)

It seems you're asking for the old behavior in this issue because it was more convenient. The only problem is, if I bring it back, we can't fix the memory leak without crashing the game. Even if it's a small memory leak, it's still not a good thing to have memory leaks.

[qaisjp]

I think there was just a language barrier here and that the issue is actually valid (I have no idea if it's fixable though).

By "lag" he means that there's a 1 frame delay between freeing a model and it actually being reclaimable via engineRequestModel. He doesn't mean any actual lag or slowdown.

In other words, he says engineRequestModel should be able to request models that were freed in the same frame.

For example, if you have a "texture packs" system, and a texture pack uses up all the available model IDs, it will not be possible to free all those IDs and re-request them in the same frame, because engineFreeModel only "marks" the IDs as reclaimable at the end of the frame.

But now if I hadn't changed the behavior and fixed the memory leak, the game would crash because there would be dangling pointers.

Does this mean that it's impossible to fix? If so, I think it's reasonable to update engineFreeModel on the wiki to say that "models are only freed at the end of the frame".

It was already documented on the wiki that the order can change. I don't see why it's a problem.

I think he was just presenting the underlying behaviour by showing that model IDs {0, 1, 2, 3} are unclaimable in the same frame.

[ghost]

I think there was just a language barrier here and that the issue is actually valid (I have no idea if it's fixable though).

By "lag" he means that there's a 1 frame delay between freeing a model and it actually being reclaimable via engineRequestModel. He doesn't mean any actual lag or slowdown.

Ah, I see. It is possible to fix it. We can take a look at how MTA's streamer works. There's a fixed size for streamed in peds (140) and if you create a ped while the memory pool is full, then MTA will stream out one of the peds and stream in the new ped in the next frame. So the point is we can do the same for the model ids. The code logic doesn't need to be complicated, we can keep it simple. My to-do list is already big enough, I'm not sure if I can do it by this month.

[TheNormalnij]

We can take a look at how MTA's streamer works

Yeah i did it. Original crash happens because MTA keep peds (and objects) with invalid model id in GTA SA pool. MTA keep invalid peds because engineFreeModel can't change model to safe model for peds because peds are not in streaming pool. Resource destructor removed peds from streaming pool and put elements in CElementDeleter list. CElementDeleter will remove peds from GTA SA pool later. So in my vision we need to remove all deleted elements from GTA SA pool before CClientModel will delete link to model info in CModelInfoSAInterface::ms_modelInfoPtrs

You fix add smart pointers for all elements to CClientModel struct. So CClientModel destructor will be called after freeing CElementDeleter list. It's delay reason

Lag

> He doesn't mean any actual lag or slowdown. Yes, I call lag any delay. [Input lag](https://en.wikipedia.org/wiki/Input_lag) also. It's better to use accurate terms.