dvdvideo1234 / TrackAssemblyTool

A Garry's mod tool for assembing a prop-segmented track
GNU Affero General Public License v3.0
21 stars 2 forks source link

Reduce the calls to `util.IsValidModel()` and use cached file checks #55

Closed dvdvideo1234 closed 5 months ago

dvdvideo1234 commented 6 months ago

https://github.com/SligWolf/sw_base/blob/main/lua/sligwolf_addons/base_library/position.lua https://github.com/SligWolf/sw_base/blob/main/lua/sligwolf_addons/base_library/util.lua#L66

dvdvideo1234 commented 6 months ago

General idea:

DB will report !1 and vector of V=0,0,0 can be assigned to the cashe. Later on spawn must extract POA from the attachment 1 of the track piece and update the vector V for the SQL cashe being used when spawning. This must happen on triggering the transform calculation. Next time someone picks the same model it will pull the POA from the cashe and snap the piece regularly.

dvdvideo1234 commented 6 months ago

On the other hand the entity is cached and one instance is used for registering all entries:

  local ePiece = GetOpVar("ENTITY_TRANSFORMPOA")
  if(ePiece and ePiece:IsValid()) then -- There is basis entity then update and extract
    if(ePiece:GetModel() ~= sModel) then ePiece:SetModel(sModel)
      LogInstance("Update "..GetReport2(ePiece:EntIndex(), sModel)) end
  else -- If there is no basis need to create one for attachment extraction
    ePiece = MakeEntityNone(sModel); if(not (ePiece and ePiece:IsValid())) then
      LogInstance("Basis creation error "..GetReport(sModel)); return nil end
    SetOpVar("ENTITY_TRANSFORMPOA", ePiece) -- Register the entity transform basis
  end -- Transfer the data from the transform attachment location
Grocel commented 6 months ago

I don't know what's so hard to get about it, to be honest. I know that English isn't exactly your strength, but we had this topic multiple times already.

It is not about the entity count. It is about the count of different models being loaded, even they might not be needed during the session. Each model being loaded stays in engine memory and takes up a slot.

Yes, even if the entity is deleted and is never visible. Models are never unloaded until you disconnect from the game session.

Just don't use dummy entities. It is bad design if you can avoid it. Imagine someone has like 20 addon with 100 models each. It will crash the game, because of a badly scaling design choice will load each of these model once until the game runs out of model slots. It affects loading times as well.

Your first idea you had earlier sounded fine enough.

dvdvideo1234 commented 6 months ago
  1. I saw I have about 9 calls of util.IsValidModel and currently do not know if replacing each and every one of these withfile.Exists will worth the effort. Some users were having problems in the past because of me not using util.IsValidModel and TA was reporting invalid models for actual valid model paths.
  2. For the dummy entity I do not think this is too much of a hassle to have only one entity here.... It can always be removed after all database records are synced and all the attachment points extracted. Currently the problem is that it will require already spawned entity to extract the attachments for the holder's model and such is not present when I call GetEntitySpawn:

    -- Found the active point ID on trEnt. Initialize origins
    stSpawn.BPos:SetUnpacked(trPOA.O[cvX], trPOA.O[cvY], trPOA.O[cvZ]) -- Read origin
    stSpawn.BAng:SetUnpacked(trPOA.A[caP], trPOA.A[caY], trPOA.A[caR]) -- Read angle
    stSpawn.BPos:Rotate(stSpawn.TAng); stSpawn.BPos:Add(stSpawn.TOrg)
    stSpawn.BAng:Set(trEnt:LocalToWorldAngles(stSpawn.BAng))
    
    -- Read holder record
    stSpawn.HPnt:SetUnpacked(hdPOA.P[cvX], hdPOA.P[cvY], hdPOA.P[cvZ])
    stSpawn.HOrg:SetUnpacked(hdPOA.O[cvX], hdPOA.O[cvY], hdPOA.O[cvZ])
    stSpawn.HAng:SetUnpacked(hdPOA.A[caP], hdPOA.A[caY], hdPOA.A[caR])

    The second problem is that if I put this behavior in MakePiece, the spawn position and angle are already passed down to the function arguments and this will not cover every use case so the only thing that is currently available as an option is spawn a dummy piece, update the transform then spawn the real piece. I do not like this resolution.. The third option is to just remove this dummy entity once every database entry is updated in case of using POA attachments. It is only one entity so removing it will be easy... Use it then delete it... Problem solved :)

The idea is nice, don't get me wrong but I have to think about it how to implement it. Currently removing the dummy entity after the initialization will be the easiest, since it is only one entity and the impact is minuscule ( use it to populate the DB, then remove it... Done ;) ). I hope I do not have to overdo the whole script ;) What do you thing about this ?

Also.. What's wrong with my english :D ?!

Grocel commented 6 months ago

Do you know what I mean with model cache in this context? It's an engine thing and it is related to this: https://gmodwiki.com/util.PrecacheModel

Every time an entity gets a model set or util.IsValidModel is used on a model, it will be added to that cache and it will stay there taking up a slot.

  1. According to wiki util.IsValidModel (https://gmodwiki.com/util.IsValidModel) might not work on models not yet precached on the server, even for client only models. You might want to use a combination if file.Exists and IsUselessModel (https://gmodwiki.com/Global.IsUselessModel). Both function are filename based and will not parse and load the model into the engine's cache. You can use an Lua cache table for your custom model exist check too. The function util.IsValidModel can be used when the user actually tries to spawn/use the model. This should not be used when the DB is read/populated, as it would have the same issues as using the dummy entity.

  2. The entity is not the problem. Setting the models on it is, when you do it for many different models. When a model is set, the model is parsed and cached by the engine (see above). However you still can use the dummy entity technique, when: I.) Right before the model is actually going to be used or II.) for a model that has already been spawned in by a player. Case I.) would be the new piece and case II.) would be the target piece where the new piece would be attached to. For II.) the dummy might not be needed every time, because positions and angles might have been loaded by the tool already. With that new idea you can keep the dummy entity, but please use it less often. Use it on demand and not globally across each DB item. That will scale much better across larger datasets then the current implantation. The new system could theoretically support infinite amounts of models without increasing the load time much or crashing the game. (Remember: "Scale" as in "scalability" in computer science, not "model scale".)

  3. Deleting the dummy entity after using it is something I would take for granted. It should be done after using it in every algorithm you like to implemented. I did not expected that it would be something to discuss. Deleting the entity will not fix the issues caused by mass model reading/caching. But it is still a good idea to delete the dummy after use.

  4. About the English. Please don't get me wrong, but I think you might have a hard time reading and fully comprehending my arguments/communication. Like they were lost in (machine) translation. I noticed that same goes for me for some of your posts to be honest too. Given the differences between our home countries/languages, it isn't even that unlikely to happen. It seems arguments are kept getting missed, even after clarification attempts. Theory of mind might be another issue, but that's something different and I am not sure about this one yet.

dvdvideo1234 commented 6 months ago
  1. Yeah... I noticed this a while ago and I wanted to write asmlib.IsModel or something that will internally use the three. This is actually a good idea. IsValidModel to be used if true is passed for example and if it is truly needed.
  2. Ah, that's the case then.. But what if it never have to set the wrong or invalid model in the first place ( use the idea from pint 1 ) .. Like ever ( as it works now for the dummy entity but call the function from point 1 instead ). I do not know is this also applies on regular entities, but I am 95% sure it does.
  3. Here however lies another problem. I'd like to support SQL mode for the database. In this case the internal library cashing is used in the most hot function function CacheQueryPiece(sModel) where every record stays in the cashe for a given amount of time. I extract the transform only when when the model is searched for in the lua cashe and not found. Then I run the transform extraction when it is needed. One idea is to make the dummy and populate all the SQL !1 and !2 then remove it. If the model is invalid, just populate NULL and call it done ;) TABLE:Record will vcreate line with !1 and !2 but on invalid model check will insert NULL.

How does this sound ?

Grocel commented 6 months ago
  1. Sure go ahead with asmlib.IsModel(). I think you should separate the concepts of models and entities in your mind more. I think that will help you to understand what I was trying to tell you the past 2 days.
  2. When done properly you would have to worry about about having to use an invalid model. The dummy entity operation would be done after the player picked the model and is about to spawn or select it in the tool. In this case you can protect with a full model validation check, because the model is going to be used right now anyways.
  3. Don't transform the attachments ids/names on database level yet, just store them as is. Transform it when you spawn/select the piece, you can use a Lua cache if needed, so you don't need to translate the attachments more then once per session. Models can change their attachment position across addon updates. Using attachments ids/names makes things more flexible for addon makers. Please don't destroy that with a bad software/database design.

By the way: It is not "cashing", it's "caching". :D Mind the letters SH and CH. Cashing is the act of being payed by someone. Caching is storing data for faster operation for next access.