[PartialCloning]

All the natives that set models such as entity_set_model and cs_set_user_model allocate a string each time they're called. Can we get new natives that allow us to provide the allocated strings?

One to work alongside entity_set_model/EngFunc_SetModel, and another to set player models including the player model persistence found in cs_set_user_model?

[addons_zz]

Originally Posted by PartialCloning All the natives that set models such as entity_set_model and cs_set_user_model allocate a string each time they're called. Can we get new natives that allow us to provide the allocated strings?

Where exactly you say it is allocating a new string?
Are you talking about here:

// native cs_set_user_model(index, const model[], bool:update_index = false);
static cell AMX_NATIVE_CALL cs_set_user_model(AMX *amx, cell *params)
{
    int index = params[1];
    int model = params[2];

    CHECK_PLAYER(index);
    edict_t *pPlayer = MF_GetPlayerEdict(index);

    if (model == -1)
    {
        MF_LogError(amx, AMX_ERR_NATIVE, "Invalid model %d", params[2]);
        return 0;
    }

    int length;
    const char *newModel = MF_GetAmxString(amx, params[2], 0, &length);

    if (!*newModel)
    {
        MF_LogError(amx, AMX_ERR_NATIVE, "Model can not be empty");
        return 0;
    }

    Players[index].SetModel(newModel);
    Players[index].UpdateModel(pPlayer);

    if (*params / sizeof(cell) >= 3 && params[3] != 0)
    {
        if (!Server)
        {
            MF_Log("cs_set_user_model is disabled with update_index parameter set");
            return 0;
        }

        GET_OFFSET("CBasePlayer", m_modelIndexPlayer);

        // Do you want to provide "models/player/%s/%s.mdl" full formatted,
        // instead of letting the native to allocate new one to create the right path?
        char modelpath[260];
        ke::SafeSprintf(modelpath, sizeof(modelpath), "models/player/%s/%s.mdl", newModel, newModel);

        for (size_t i = 0; i < HL_MODEL_MAX; ++i)
        {
            if (Server->model_precache[i] && !strcmp(Server->model_precache[i], modelpath))
            {
                if (pPlayer->v.modelindex != i)
                {
                    SET_MODEL(pPlayer, STRING(ALLOC_STRING(modelpath)));
                }

                set_pdata<int>(pPlayer, m_modelIndexPlayer, i);
                return 1;
            }
        }

        MF_Log("Model must be precached using cs_set_user_model with update_index parameter set");
        return 0;
    }

    return 1;
}

[Arkshine]

Originally Posted by PartialCloning All the natives that set models such as entity_set_model and cs_set_user_model allocate a string each time they're called. Can we get new natives that allow us to provide the allocated strings? One to work alongside entity_set_model/EngFunc_SetModel, and another to set player models including the player model persistence found in cs_set_user_model?

It's not related only to model, there are more natives which allocate the string through the engine allocator. Having new natives which take an index doesn't seem a good idea. Such memory pool is not freed at map change, and that's not really the role of a plugin to do that. This would add some unnecessary complexity.

A better solution would be globally keeping track of allocated strings. ReHLDS actually did improve this engine function by essentially creating a new fixed memory pool (freed at mapchange) and keeping track of allocated strings in a hashmap.

For AMXX, I guess we could just create a function wrapping this engine function around an hashmap and then using it everywhere it's needed. Adding it in AMXX API for use with modules as well.

[PartialCloning]

Originally Posted by addons_zz ..

SET_MODEL(pPlayer, STRING(ALLOC_STRING(modelpath)));

Originally Posted by Arkshine ..

Most of the posts addressing string allocation say it's an expensive operation. Wouldn't it be better for the plugin to allocate the string one time and use it whereever it needs it? Even in the case of ReHLDS, rather than allocating the string all the time which can be pretty often when replacing weapon models.

Would the new AMXX function only allocate the string once even across map changes? Should plugins switch any natives that require allocated strings like EngFunc_CreateNamedEntity to natives that allocate the strings on their own like create_entity?

[Arkshine]

If don't say bullshits, performance-wise ALLOC_STRING is not called that often and the engine memory pool should be already allocated anyway, therefore it's unlikely there an issue here. That would be more to save memory.

If something is done in AMXX, it will be likely global and transparent (same native, nothing to do). Assuming you're using the original engine, then strings are stored until HLDS stops.

[addons_zz]

Originally Posted by Arkshine It's not related only to model, there are more natives which allocate the string through the engine allocator. Having new natives which take an index doesn't seem a good idea. Such memory pool is not freed at map change, and that's not really the role of a plugin to do that. This would add some unnecessary complexity.

Or the 'ALLOC_STRING' string calls currently not freeing their allocated memory on map change (very very bad) or your forgot that users would need to call AllocStringDestroy( id ), as for tries, etc.
However, every time the AMXX do ALLOC_STRING and return the ID to use around, the AMXX could keep them on simple list, and loop throw them on map change deleting them. But of course this is not the solution's part we want to.
This answer my question. They are not freed map change.

Originally Posted by Arkshine If something is done in AMXX, it will be likely global and transparent (same native, nothing to do). Assuming you're using the original engine, then strings are stored until HLDS stops.

Originally Posted by Arkshine For AMXX, I guess we could just create a function wrapping this engine function around an hashmap and then using it everywhere it's needed. Adding it in AMXX API for use with modules as well.

This looks a good solution, because hide the complexity from the AMXX programmers then they can spend more time developing their code instead of worry about engine optimizations.

Originally Posted by PartialCloning Most of the posts addressing string allocation say it's an expensive operation. Wouldn't it be better for the plugin to allocate the string one time and use it whereever it needs it? Even in the case of ReHLDS, rather than allocating the string all the time which can be pretty often when replacing weapon models.

You do not understand what @Arkshine said. Yes, his solution already solves it. Now you can code it yourself and to do a pull request on github. There are few more details like, if the engine on delete them only on close, you would have to keep the hashtable alive across mapchanges. But it is only problem if the engine re-allocates the string x, every time the x allocation is called, and keep both on memory until the server closes.

Originally Posted by Arkshine For AMXX, I guess we could just create a function wrapping this engine function around an hashmap and then using it everywhere it's needed. Adding it in AMXX API for use with modules as well.

Originally Posted by PartialCloning Would the new AMXX function only allocate the string once even across map changes?

Wow, no way even across map changes. This is called memory leak, and it helps you HLDS eat memory until it explodes and crash your server.
Really, it will not kill you allocate the string only one time per map. And actually, there are no new functions on the presented solution, just the current natives which call ALLOC_STRING, will be fixed.

[Arkshine]

I have no idea why you're saying. Looks like you're confusing a lot of things.

[addons_zz]

Then the problem is not about memory leaking but about the cost to call the engine function to allocate string and return a pointer to it.

[Arkshine]

There should have no cost, or very trivial.

[PartialCloning]

Lets say we're setting the knife model on item deploy.

Which method should be used for:
1. Current version of amx.
2. If the new function is eventually implemented.
3. ReHLDS.

set_pev(id, pev_viewmodel2, "models/v_newknife.mdl");

static alloc;
if(alloc || (alloc = engfunc(EngFunc_AllocString, "models/v_newknife.mdl")))
    set_pev_string(id, pev_viewmodel2, alloc);