Added net.WriteCompressed() and net.ReadCompressed()

[EstevanTH]

Hi Willox, hi Robotboy655, hi everyone!

In order to give access to compression in net messages for little experienced Lua developers, I am giving an easy way to send strings (text or binary) using compression.

To achieve this, I have created the functions net.WriteCompressed() and net.ReadCompressed().

Hoping the best,

Momo :heart:

[Kefta]

There's no need to call WriteData if the length is 0.

[EstevanTH]

I guessed this, thanks for the confirm. To make the code as simple as possible, I deliberately chose to call it without any check, and that rocks! 😋

[Kefta]

It adds unnecessary data to the buffer, though. You also aren't reading data you're writing, which can lead to misalignments.

[neico]

Just move the data size check to a if condition and do nothing if there's no data to begin with.

Also: #data without checking for validity is dangerous as #nil will cause an error that might not be immediately visible to the end user of why it's happening, better is data and #data ~= 0

[Kefta]

@neico To be fair, datatype checking is pretty inconsistent across the GMod API overall, but I agree something like:

assert( isstring( data ), "net.WriteCompressed: string expected, got " .. type( data ) )

would work.

[EstevanTH]

I have fixed my pull request by using your recommendations. Thank you!

[Kefta]

I would remove the function localisations. It's not really consistent with the rest of the file.

[EstevanTH]

I'm letting the decision to Facepunch staff. In my opinion localizing functions is safe and efficient while using non-localized functions has no benefit besides the fact that they are not local. 😛

[neico]

I'd probably write it as follows, but I'm not sure if that makes it any faster or less complex :/

function net.WriteCompressed( data )
	assert( isstring( data ), "net.WriteCompressed: string expected, got " .. type( data ) )

	local length = #data
	local compressed
	if length > 0 then -- Is `not equal` faster than `greater than` when used on numbers?
		compressed = util_Compress( data )
		length = #compressed
	end

	net_WriteUInt( length, 16 )
	if compressed then net_WriteData( compressed, length ) end
	return length
end

[Kefta]

@neico Both > 0 and ~= 0 are a single processor instruction.

I personally would write it as:

function net.WriteCompressed( data )
	assert( isstring( data ), "net.WriteCompressed: string expected, got " .. type( data ) )
	
	if ( #data == 0 ) then
		net.WriteUInt( 0, 16 )
	else
		local compressed = util.Compress( data )
		local length = string.len( compressed )
		net.WriteUInt( length, 16 )
		net.WriteData( compressed, string.len( compressed ) )
	end
end

function net.ReadCompressed()
	local length = net.ReadUInt( 16 )
	
	if ( length == 0 ) then
		return ""
	end
	
	return util.Decompress( net.ReadData( length ) )
end

It is the most optimised while fitting the general style of the surrounding code.

EDIT: Changed string.len to #

[neico]

I'm pretty sure # is faster than string.len tho (think of the field lookup and function call overhead), besides, I'd use compressed:len() etc. instead

but that's getting nit-picky now...

[Kefta]

Doesn't # just call __len, which then calls string.len? Also, the meta version would definitely be the slowest: it has to call __index then string.len.

Good thing to benchmark, I suppose.

[bigdogmat]

__len doesn't work on strings

[Kefta]

I guess it's an internal Lua change, then

[EstevanTH]

I assumed without any test that #myString is faster than string.len( myString ). But my results do not show any significant difference even though neither string or string.len() were referenced as local.

Here are my results:

#myString -	5.5318379227713
string.len( myString ) -	5.5382541087758
myString:len() -	5.5253148003335

[ERROR] lua/autorun/mon_benchmark.lua:44: attempt to index a string value with bad key ('__len' is not part of the string library)
  1. error - [C]:-1
   2. __index - lua/includes/extensions/string.lua:297
    3. mon_benchmark - lua/autorun/mon_benchmark.lua:44
     4. unknown - lua_run:1

function mon_benchmark()
	local times = 4294967295
	
	local f = file.Open( "maps/gm_construct.bsp", "rb", "GAME" )
	local myString = f:Read( f:Size() )
	f:Close()
	f = nil
	
	local SysTime = SysTime
	local start = 0.
	local dummy = 0
	local finish = 0.
	
	SysTime()
	
	-- Test 1
	start = SysTime()
	for i = 1,times do
		dummy = #myString
	end
	finish = SysTime()
	print( "#myString -", finish-start )
	
	-- Test 2
	start = SysTime()
	for i = 1,times do
		dummy = string.len( myString )
	end
	finish = SysTime()
	print( "string.len( myString ) -", finish-start )
	
	-- Test 3
	start = SysTime()
	for i = 1,times do
		dummy = myString:len()
	end
	finish = SysTime()
	print( "myString:len() -", finish-start )
	
	-- Test 4
	start = SysTime()
	for i = 1,times do
		dummy = myString:__len()
	end
	finish = SysTime()
	print( "myString:__len() -", finish-start )
end

[Kefta]

The operations are being optimised out by JIT, which explains the wacky time differences. @neico was correct - the len operator is significantly more efficient.

Control: 10.504402196939
#str: 10.656063404538
string_len: 15.770884548088

Code:

jit.off()

local string_len = string.len
local SysTime = SysTime
local print = print

local File = file.Open("maps/gm_construct.bsp", "rb", "GAME")
local sDummy = File:Read(File:Size())
File:Close()
File = nil

local fDummy = function() end
local flStart = 0
local flEnd = 0

flStart = SysTime()

for i = 999999999, 0, -1 do
	fDummy(sDummy)
end

flEnd = SysTime()
print("Control: " .. flEnd - flStart)

flStart = SysTime()

for i = 999999999, 0, -1 do
	fDummy(#sDummy)
end

flEnd = SysTime()
print("#str: " .. flEnd - flStart)

flStart = SysTime()

for i = 999999999, 0, -1 do
	fDummy(string_len(sDummy))
end

flEnd = SysTime()
print("string_len: " .. flEnd - flStart)

jit.on()

[bigdogmat]

I guess it's an internal Lua change, then

It's how it is for all versions of Lua.

[h3xcat]

@Kefta net.WriteData with length of 0 doesn't add data to buffer.

[robotboy655]

I fear this will just enable bad actors to much more easily send zipbombs to any server using this, like they already do with dupes.