Compressor: Use minCompressionThreshold exclusively

closes #5589

src/Server.php

@@ -1388,7 +1388,8 @@ class Server{
 			$buffer = $stream->getBuffer();
 
 			if($sync === null){
-				$sync = !($this->networkCompressionAsync && $compressor->willCompress($buffer));
+				$threshold = $compressor->getCompressionThreshold();
+				$sync = !$this->networkCompressionAsync || $threshold === null || strlen($stream->getBuffer()) < $threshold;
 			}
 
 			$promise = new CompressBatchPromise();

src/network/mcpe/StandardPacketBroadcaster.php

@@ -23,7 +23,6 @@ declare(strict_types=1);
 
 namespace pocketmine\network\mcpe;
 
-use pocketmine\network\mcpe\compression\ThresholdCompressor;
 use pocketmine\network\mcpe\protocol\serializer\PacketBatch;
 use pocketmine\network\mcpe\protocol\serializer\PacketSerializer;
 use pocketmine\Server;
@@ -64,36 +63,23 @@ final class StandardPacketBroadcaster implements PacketBroadcaster{
 			foreach($compressorMap as $compressorId => $compressorTargets){
 				$compressor = $compressors[$compressorId];
 
-				$batchBuffer = null;
+				$threshold = $compressor->getCompressionThreshold();
-				if($compressor instanceof ThresholdCompressor){
+				if($threshold !== null && $packetBufferTotalLengths[$bufferId] >= $threshold){
-					$threshold = $compressor->getCompressionThreshold();
+					//do not prepare shared batch unless we're sure it will be compressed
-					if($threshold !== null && $packetBufferTotalLengths[$bufferId] >= $threshold){
-						//do not prepare shared batch unless we're sure it will be compressed
-						$stream = new BinaryStream();
-						PacketBatch::encodeRaw($stream, $packetBuffers[$bufferId]);
-						$batchBuffer = $stream->getBuffer();
-					}
-				}else{
-					//this is a legacy compressor, so we have to encode the batch and check if it will compress
 					$stream = new BinaryStream();
 					PacketBatch::encodeRaw($stream, $packetBuffers[$bufferId]);
-					$tempBatchBuffer = $stream->getBuffer();
+					$batchBuffer = $stream->getBuffer();
-					if($compressor->willCompress($tempBatchBuffer)){
-						$batchBuffer = $tempBatchBuffer;
-					}
-				}
 
-				if($batchBuffer === null){
+					$promise = $this->server->prepareBatch(new PacketBatch($batchBuffer), $compressor);
+					foreach($compressorTargets as $target){
+						$target->queueCompressed($promise);
+					}
+				}else{
 					foreach($compressorTargets as $target){
 						foreach($packetBuffers[$bufferId] as $packetBuffer){
 							$target->addToSendBuffer($packetBuffer);
 						}
 					}
-				}else{
-					$promise = $this->server->prepareBatch(new PacketBatch($batchBuffer), $compressor);
-					foreach($compressorTargets as $target){
-						$target->queueCompressed($promise);
-					}
 				}
 			}
 		}

src/network/mcpe/compression/Compressor.php

@@ -24,13 +24,20 @@ declare(strict_types=1);
 namespace pocketmine\network\mcpe\compression;
 
 interface Compressor{
-
-	public function willCompress(string $data) : bool;
-
 	/**
 	 * @throws DecompressionException
 	 */
 	public function decompress(string $payload) : string;
 
 	public function compress(string $payload) : string;
+
+	/**
+	 * Returns the minimum size of packet batch that the compressor will attempt to compress.
+	 *
+	 * The compressor's output **MUST** still be valid input for the decompressor even if the compressor input is
+	 * below this threshold.
+	 * However, it may choose to use a cheaper compression option (e.g. zlib level 0, which simply wraps the data and
+	 * doesn't attempt to compress it) to avoid wasting CPU time.
+	 */
+	public function getCompressionThreshold() : ?int;
 }

src/network/mcpe/compression/ThresholdCompressor.php

@@ -1,31 +0,0 @@
-<?php
-
-/*
- *
- *  ____            _        _   __  __ _                  __  __ ____
- * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
- * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
- * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
- * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Lesser General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * @author PocketMine Team
- * @link http://www.pocketmine.net/
- *
- *
- */
-
-declare(strict_types=1);
-
-namespace pocketmine\network\mcpe\compression;
-
-interface ThresholdCompressor extends Compressor{
-	/**
-	 * Returns the minimum size of packet batch that will be compressed. Null means that no packets will be compressed.
-	 */
-	public function getCompressionThreshold() : ?int;
-}

src/network/mcpe/compression/ZlibCompressor.php

@@ -32,7 +32,7 @@ use function zlib_decode;
 use function zlib_encode;
 use const ZLIB_ENCODING_RAW;
 
-final class ZlibCompressor implements ThresholdCompressor{
+final class ZlibCompressor implements Compressor{
 	use SingletonTrait;
 
 	public const DEFAULT_LEVEL = 7;
@@ -56,10 +56,6 @@ final class ZlibCompressor implements ThresholdCompressor{
 		return $this->minCompressionSize;
 	}
 
-	public function willCompress(string $data) : bool{
-		return $this->minCompressionSize !== null && strlen($data) >= $this->minCompressionSize;
-	}
-
 	/**
 	 * @throws DecompressionException
 	 */
@@ -76,11 +72,12 @@ final class ZlibCompressor implements ThresholdCompressor{
 	}
 
 	public function compress(string $payload) : string{
+		$compressible = $this->minCompressionSize !== null && strlen($payload) >= $this->minCompressionSize;
 		if(function_exists('libdeflate_deflate_compress')){
-			return $this->willCompress($payload) ?
+			return $compressible ?
 				libdeflate_deflate_compress($payload, $this->level) :
 				self::zlib_encode($payload, 0);
 		}
-		return self::zlib_encode($payload, $this->willCompress($payload) ? $this->level : 0);
+		return self::zlib_encode($payload, $compressible ? $this->level : 0);
 	}
 }