wiremod / wire

Garry's Mod add-on that allows users to wire up components in order to make more elaborate automatic and user-controlled contraptions.
http://www.wiremod.com
Apache License 2.0
551 stars 332 forks source link

Wiremod usermessages to Net Library #946

Closed Mantas-2155X closed 5 years ago

Mantas-2155X commented 9 years ago

How about converting wiremod's usermessages to net library? if you use EGP alot, there will be a crap ton of usermessages. Maybe change to net library?

oldmud0 commented 9 years ago

What are the performance improvements?

thegrb93 commented 9 years ago

http://facepunch.com/showthread.php?t=1304676

I've had enormous server/network lag from some egp games some people like to make. Net messages can help that, but something needs to be put in place to limit the rate of data sent or reliable channel overflows can occur.

Nebual commented 9 years ago

I believe EGP does actually use Net messages, https://github.com/wiremod/wire/blob/master/lua/entities/gmod_wire_egp/lib/egplib/umsgsystem.lua , at least in most places, however it still sends 1 Net message per instruction.

You could definitely save some performance by switching EGP's use of packets to a more batch style, where it groups all requests in a single frame and then sends them as a single Net packet. Many parts of wire do this already.

Divran commented 9 years ago

Wiremod already uses net messages pretty much everywhere where it matters.

EGP already uses net messages, and it already tries to put as much data as it can in each message before sending it and starting a new one. When I made EGP, I tested it a lot to make sure the default transfer rates would not cause lag even if you spammed like shit. I tested it by creating 300 boxes and then removing them all (over and over) as fast as the transfer rates would allow. In my local server, I could set the transfer rates to 10 times faster what the defaults were without any lag at all, which leaves a large margin for people running on potatoes.

Someone else updated EGP to use net messages. Maybe that person did not properly make sure the default transfer rates didn't cause lag? I'm pretty sure they did though.

thegrb93 commented 9 years ago

It was a long time ago I noticed it so could be different now.

Mantas-2155X commented 9 years ago

EGP_HUD_Use: EGP_ScrWH_Request: HUDIndicatorFactor: HUDIndicatorHideHUD: HUDIndicatorRegister: HUDIndicatorStyleFullCircle: HUDIndicatorStylePercent: HUDIndicatorUnRegister: RcvEntityVarBeam_Angle: RcvEntityVarBeam_Bool: RcvEntityVarBeam_Entity: RcvEntityVarBeam_Float: RcvEntityVarBeam_Int: RcvEntityVarBeam_String: RcvEntityVarBeam_Vector: UpdateEyePodState: WireMapInterfaceEnt: cpulib_debugdata_interrupt: cpulib_debugdata_registers: cpulib_debugdata_variables: e2_remoteupload_request: wire pod hud: wire_clutch_links: wire_gpu_memorymodel: wire_gpu_monitorstate: wire_gpulib_setent: wire_memsync: wire_spu_memorymodel: wire_spu_soundsources: wire_spu_soundstate:

Here are most of the wiremod's usermessages. Quite a few, huh? How about converting these to net msg

Divran commented 9 years ago

EGP_HUD_Use and EGP_ScrWH_Request send less than 255 bytes of data, so converting them to net message is pointless. Same goes for e2_remoteupload_request. Most likely wire pod hud as well. I don't know about the others.

Mantas-2155X commented 9 years ago

well, it would still be better to convert them for faster usage since net messages are faster than usermessages.

Divran commented 9 years ago

The speed difference doesn't matter in those cases that I listed. It would only be worth it if net messages are more efficient (by that I mean they put less strain on the server), but I've not seen any proof of that.

Mantas-2155X commented 9 years ago

Well the usermessages spike like this: http://i.imgur.com/UXMxfaX.jpg

while using wiremod in our server.

Mantas-2155X commented 9 years ago

The GPU ones look pretty big,

    umsg.Start("wire_gpu_memorymodel")
      umsg.Long(self:EntIndex())
      umsg.Long (self.RAMSize)
      umsg.Float(self.SerialNo)
      umsg.Short(self.ChipType)
    umsg.End()

    umsg.Start("wire_gpu_monitorstate")
      umsg.Long(self:EntIndex())
      umsg.Short(#self.Monitors)
      for idx=1,#self.Monitors do
          umsg.Long(self.Monitors[idx])
      end
    umsg.End()
Divran commented 9 years ago

I think net messages might show up on the net graph as usermessages, too. In any case, it's not possible for the ones I listed to cause such lag, since they are very rarely used. However I can't speak for the GPU/CPU ones since I don't know how those work.

Those might look big, but they're still obviously less than 255 bytes, since they only use a single usermessage.

Mantas-2155X commented 9 years ago

No, net messages don't show up as usermessages in net_graph. But anyway, thank you for reviewing. If you ever think of converting them to net messages, please do so!

TomyLobo commented 9 years ago

wire_gpu_monitorstate looks like it would overflow the packet if you have like 60-ish GPUs

TomyLobo commented 9 years ago

https://github.com/TomyLobo/picker/blob/master/lua/autorun/client/umsg_stats.lua i made this a while back if it still works, you might be able to use it to diagnose which umsg causes those spikes

Mantas-2155X commented 9 years ago

Thank you! I will check it out.

Mantas-2155X commented 9 years ago

Okay, so the issue is these usermessages: RcvEntityVarBeam_Angle RcvEntityVarBeam_Bool RcvEntityVarBeam_Entity RcvEntityVarBeam_Float RcvEntityVarBeam_Int RcvEntityVarBeam_String RcvEntityVarBeam_Vector

They are the ones that are spiking. Any ideas?

They are made just by one file: beam_netvars.lua

Divran commented 9 years ago

ugh, that file. All dependencies on that file needs to be erased and the file deleted. I believe it's only used in a few places. I'd find out, but the github search function is complete and utter crap. Let me try google's site:github.com instead, that might give better results... Nope. I can't find where those functions are used without downloading wiremod (don't have it on this computer) and using notepad++'s find in files or something. Can you do it?

Mantas-2155X commented 9 years ago

So you're saying it's safe to remove the file from my server?

just looked through it, it is only used on the beam_netvars.lua nowhere else

Divran commented 9 years ago

No. It's obviously used somewhere or you wouldn't get the spikes. I'm saying we, the wire team, should phase it out and get rid of it because it's an ancient relic

If you could just search for all the places where it's used though, that'd be great. Search for SetNetworkedBeam and SetNWB and you should find them. If not I'll try searching when I get home or whatever

Mantas-2155X commented 9 years ago

Okay, I'm looking through all wiremod's lua files and I found this..

Search "SetNetworkedBeam" (31 hits in 6 files)
  C:\Users\**\Desktop\wire\lua\entities\gmod_wire_graphics_tablet.lua (1 hit)
    Line 96:    self:SetNetworkedBeamBool("draw_background", draw_background, true)
  C:\Users\**\Desktop\wire\lua\entities\gmod_wire_screen.lua (2 hits)
    Line 7:     self:SetNetworkedBeamFloat( 1, float, true )
    Line 11:    self:SetNetworkedBeamFloat( 2, float, true )
  C:\Users\**\Desktop\wire\lua\weapons\gmod_tool\stools\wire.lua (3 hits)
    Line 192:               self.CurrentComponent:SetNetworkedBeamString("BlinkWire", self.CurrentInput)
    Line 341:       self.CurrentComponent:SetNetworkedBeamString("BlinkWire", "")
    Line 393:       self.CurrentComponent:SetNetworkedBeamString("BlinkWire", self.CurrentInput)
  C:\Users\**\Desktop\wire\lua\weapons\gmod_tool\stools\wire_adv.lua (4 hits)
    Line 318:       if IsValid(self.CurrentEntity) then self.CurrentEntity:SetNetworkedBeamString("BlinkWire", "") end
    Line 636:               ent:SetNetworkedBeamString("BlinkWire", check[self.CurrentWireIndex][1])
    Line 708:               self.AimingEnt:SetNetworkedBeamString("BlinkWire", "")
    Line 729:                       ent:SetNetworkedBeamString("BlinkWire", check[self.CurrentWireIndex][1])
  C:\Users\**\Desktop\wire\lua\wire\beam_netvars.lua (4 hits)
    Line 10: // Just like NetVars, ent:SetNetworkedBeam*        //
    Line 157: // make all the ent.Get/SetNetworkedBeamVarCrap
    Line 166:   meta[ "SetNetworkedBeam" .. name ] = function ( self, key, value, urgent )
    Line 192:   meta[ "SetNWB" .. name ] = meta[ "SetNetworkedBeam" .. name ]
  C:\Users\**\Desktop\wire\lua\wire\server\wirelib.lua (17 hits)
    Line 589:   ent:SetNetworkedBeamInt(net_name, 0)
    Line 590:   ent:SetNetworkedBeamVector(net_name .. "_start", pos)
    Line 591:   ent:SetNetworkedBeamString(net_name .. "_mat", material)
    Line 592:   ent:SetNetworkedBeamVector(net_name .. "_col", Vector(color.r, color.g, color.b))
    Line 593:   ent:SetNetworkedBeamFloat(net_name .. "_width", width)
    Line 613:   CurLink[idx].Dst:SetNetworkedBeamEntity(net_name .. "_" .. node_idx .. "_ent", ent)
    Line 614:   CurLink[idx].Dst:SetNetworkedBeamVector(net_name .. "_" .. node_idx .. "_pos", pos)
    Line 615:   CurLink[idx].Dst:SetNetworkedBeamInt(net_name, node_idx)
    Line 663:   CurLink[idx].Dst:SetNetworkedBeamEntity(net_name .. "_" .. node_idx .. "_ent", ent)
    Line 664:   CurLink[idx].Dst:SetNetworkedBeamVector(net_name .. "_" .. node_idx .. "_pos", pos)
    Line 665:   CurLink[idx].Dst:SetNetworkedBeamInt(net_name, node_idx)
    Line 693:       CurLink[idx].Dst:SetNetworkedBeamEntity(net_name .. "_" .. i, CurLink[idx].OldPath[i].Entity)
    Line 694:       CurLink[idx].Dst:SetNetworkedBeamVector(net_name .. "_" .. i, CurLink[idx].OldPath[i].Pos)
    Line 697:   CurLink[idx].Dst:SetNetworkedBeamInt(net_name, path_len)
    Line 705:   ent:SetNetworkedBeamInt(net_name, 0)
    Line 713:       ent:SetNetworkedBeamString("wpn_" .. k, v)
    Line 715:   ent:SetNetworkedBeamInt("wpn_count", #names)
Mantas-2155X commented 9 years ago

http://i.imgur.com/ggITnRp.jpg

TomyLobo commented 9 years ago

@Divran don't forget printing it out and burning the paper

AbigailBuccaneer commented 9 years ago

After #956 is pulled, the only thing that Wiremod still uses beam vars for is the wire paths. They don't change very often, but there's a lot of data in them. Replacing wire paths with the net library won't be a trivial task.

In particular, one thing the beam library does nicely is that it sends all entity data slowly when a client connects. Having a centralised system to do that does make a lot of sense, and if I were to switch everything to use NWVars then new player join times might be unreasonably long.

Still, I suspect a small custom system (using the net library rather than usermessages) just for wire paths would be quite a bit more efficient.

Divran commented 9 years ago

In particular, one thing the beam library does nicely is that it sends all entity data slowly when a client connects.

Thing is, net messages are so huge that the odds you'd ever need more than a single one to send all the data is pretty small.

Mantas-2155X commented 9 years ago

okay i took the files @AbigailBuccaneer edited and added them to my server. It seems it's better now. But some stuff still do it.

TomyLobo commented 9 years ago

can't you just move beam to net with the messages they way they are?

Divran commented 9 years ago

@TomyLobo that would be an enormous waste and we would gain absolutely nothing

EDIT: Unless you also meant to edit the amount of data sent in each message, then it might work.

TomyLobo commented 9 years ago

no I meant without touching those well-tested numbers. and we would gain a more responsive reliable channel

AbigailBuccaneer commented 9 years ago

@TomyLobo that is true, it would be a partial solution. But the wire path messages are highly-structured data that's being sent really inefficiently as is, and as it's the only thing remaining using the beam library I think it makes sense to just replace it with something smaller and more specialised.

Divran commented 9 years ago

@TomyLobo If we don't change the values, we'd get the same amount of messages sent, just using a different library to send them. So the question is: what causes more lag? One large single message, or hundreds of tiny ones? I believe hundreds of tiny ones causes more lag, which is why I think it's a bad idea to simply change umsg to net and change nothing else.

In fact, it might be better to queue all the data and send it every 5 seconds to everyone. That way we can get even fewer (and larger) messages. Except for the user creating the wires. They of course want to be able to see the wires immediately, so why not take advantage of the fact that the adv wire tool is client side and simply create the wires immediately for that person? Legacy wire tool users would then need to wait up to 5 seconds for the next sync, in that case.

oldmud0 commented 9 years ago

For some reason I always had the impression that Source used TCP, but it actually uses UDP.

5 seconds seems too long for a client sync... When we fiddle with packet sizes this is latency vs. throughput.

In this case, a mixed system would work. If we expect only a few messages then it would be fine to deliver them immediately so that latency does not become an issue. But say if we're pasting a dupe, then it would be better to buffer all the messages and send them as one packet.

How about a buffer that is flushed every 500ms? That would be a good tradeoff.

Divran commented 9 years ago

@oldmud0 keep in mind that this is just for some static visual effects that don't have much impact on the game, and don't change very often, if ever. Really the only person who can't afford to wait a second or two (mostly because it'd feel laggy and unresponsive if it takes too long) is the person who is creating the wires. Everyone else can deal with it.

Another option however could be to vary the transfer rate depending on how close the user is. If the user is within 500 units to the created/modified/deleted wire, then sync it immediately. Within 5000 units, sync after 5 seconds (or immediately if they come within 500 units). Beyond that, don't sync at all (or maybe after like 30 seconds?).

TomyLobo commented 9 years ago

@AbigailBuccaneer wrote: 21:48 - abigaildanger: https://github.com/wiremod/wire/issues/946 in response to this: i've written a nice lil replacement for wire paths, should have a much much lower bandwidth use (both in size of data and number of netmessages) 21:48 - abigaildanger: but i need to review it and test it carefully

I guess we should wait for that, sounds promising

Divran commented 9 years ago

yep I saw that too

Mantas-2155X commented 9 years ago

http://imgur.com/27Lqzwe right.. so it seems the wire_memsync thing is literally spamming my server.. Any idea on this? I looked through the code, converted all the usermessages from wiremod except wire_memsync and beamvars to net messages, but this one is weird, it has all the data compress things and i really have no idea how to convert that.

TomyLobo commented 9 years ago

@WGHydrogen you're aware of the umsg_stats command? It's supposed to show averages from the past, sorted by amount. You should try that.

Mantas-2155X commented 9 years ago

@TomyLobo the umsg_stats doesn't really work..

Mantas-2155X commented 9 years ago

Anyway, I have just spent an hour working to convert the beam_netvars to net messages.. and it's a success! Now its all net messages no more usermessages. The last thing is wire_memsync.

Divran commented 9 years ago

but didn't abigail already do that

On Fri, Sep 18, 2015 at 8:50 PM, WGHydrogen notifications@github.com wrote:

Anyway, I have just spent an hour working to convert the beam_netvars to net messages.. and it's a success! Now its all net messages no more usermessages. The last thing is wire_memsync.

— Reply to this email directly or view it on GitHub https://github.com/wiremod/wire/issues/946#issuecomment-141534182.

Mantas-2155X commented 9 years ago

no. I have converted the whole thing. Not only the few things he did. like the whole system of beam_netvars.

Mantas-2155X commented 9 years ago

http://imgur.com/YaUcWgP

Divran commented 9 years ago

ah nevermind, abigail actually REMOVED the beam var library from wiremod completely (pull request not yet merged), which is even better.

oldmud0 commented 9 years ago

Could changing everything to net messages make it more likely to cause a buffer overflow in net message error? Just curious.

Mantas-2155X commented 9 years ago

I haven't seen a buffer overflow in my server yet, wiremod has been actively used and its fine. I also made it so it sends the net messages instantly, instead of delayed like previously it was with umsg (beamvars)

Divran commented 9 years ago

it's still better to simply remove the beam vars library entirely

Mantas-2155X commented 9 years ago

okay, if we did that, we would have to use networkvars right? But aren't NWVars and NetworkVars usermessages? Would it be faster to use net messages instead?

TomyLobo commented 9 years ago

Turning the beam vars lib into a shim around nwvars would give us the benefit of being able to easily switch to a new system if it makes sense

Divran commented 9 years ago

Pretty sure NWVars and NetworkVars are not usermessages. TomyLobo, that would be true if wiremod used beam vars at all, but it only uses it in like 2 places.

On Mon, Sep 21, 2015 at 7:26 AM, TomyLobo notifications@github.com wrote:

Turning the beam vars lib into a shim around nwvars would give us the benefit of being able to easily switch to a new system if it makes sense

— Reply to this email directly or view it on GitHub https://github.com/wiremod/wire/issues/946#issuecomment-141880408.

Mantas-2155X commented 9 years ago

well.. If i use any of the wiremod tools like: Screens, text screens, E2, GPU, CPU, EGP.. any wiremod tool, it sends some beamvars.