Remove slice! from fixed_protocol

openc3/lib/openc3/interfaces/protocols/fixed_protocol.rb

@@ -102,7 +102,7 @@ def identify_and_finish_packet
         end
 
         if unique_id_mode
-          target_packets.each do |packet_name, packet|
+          target_packets.each do |_packet_name, packet|
             if packet.identify?(@data[@discard_leading_bytes..-1])
               identified_packet = packet
               break
@@ -136,7 +136,13 @@ def identify_and_finish_packet
           @packet_name = identified_packet.packet_name
 
           # Get the data from this packet
-          packet_data = @data.slice!(0, identified_packet.defined_length + @discard_leading_bytes)
+          # Previous implementation looked like the following:
+          #   packet_data = @data.slice!(0, identified_packet.defined_length + @discard_leading_bytes)
+          # But slice! is 6x slower at small packets (1024)
+          # and 1000x slower at large packets (1Mb)
+          # Run test/benchmarks/string_mod_benchmark.rb for details
+          packet_data = @data[0...identified_packet.defined_length + @discard_leading_bytes]
+          @data = @data[(identified_packet.defined_length + @discard_leading_bytes)..-1]
           break
         end
       end
@@ -152,7 +158,7 @@ def identify_and_finish_packet
         @data.replace('')
       end
 
-      return packet_data
+      return packet_data, @extra
     end
 
     def reduce_to_single_packet

openc3/spec/interfaces/protocols/fixed_protocol_spec.rb

@@ -179,6 +179,47 @@ def read
         expect(packet.buffer).to eql $buffer
         target.cmd_unique_id_mode = false
       end
+
+      it "breaks apart telemetry data from the stream" do
+        packet = System.telemetry.packet("SYSTEM", "META")
+        packet.write('PKTID', 1)
+        packet.write('OPERATOR_NAME', 'RYAN')
+        $buffer1 = packet.buffer.clone
+        packet.write('OPERATOR_NAME', 'JASON')
+        $buffer2 = packet.buffer.clone
+        $buffer = "\x1A\xCF\xFC\x1D" + $buffer1 + "\x1A\xCF\xFC\x1D" + $buffer2
+        $index = 0
+
+        class FixedStream3 < Stream
+          def connect; end
+
+          def connected?; true; end
+
+          def read
+            # Send a byte a time
+            $index += 1
+            $buffer[$index - 1]
+          end
+        end
+        # Require 5 bytes, discard 4 leading bytes, use 0x1ACFFC1D sync, telemetry = true
+        @interface.add_protocol(FixedProtocol, [5, 4, '0x1ACFFC1D', true], :READ_WRITE)
+        @interface.instance_variable_set(:@stream, FixedStream3.new)
+        @interface.target_names = ['SYSTEM']
+        @interface.cmd_target_names = ['SYSTEM']
+        @interface.tlm_target_names = ['SYSTEM']
+        packet = @interface.read
+        expect(packet.received_time.to_f).to be_within(0.01).of(Time.now.to_f)
+        expect(packet.target_name).to eql 'SYSTEM'
+        expect(packet.packet_name).to eql 'META'
+        expect(packet.buffer).to include('RYAN')
+        packet = @interface.read
+        expect(packet.received_time.to_f).to be_within(0.01).of(Time.now.to_f)
+        expect(packet.target_name).to eql 'SYSTEM'
+        expect(packet.packet_name).to eql 'META'
+        expect(packet.buffer).to include('JASON')
+        packet = @interface.read
+        expect(packet).to be_nil
+      end
     end
   end
 end

openc3/test/benchmarks/string_mod_benchmark.rb

@@ -0,0 +1,50 @@
+# encoding: ascii-8bit
+
+# Copyright 2023 OpenC3, Inc.
+# All Rights Reserved.
+#
+# This program is free software; you can modify and/or redistribute it
+# under the terms of the GNU Affero General Public License
+# as published by the Free Software Foundation; version 3 with
+# attribution addendums as found in the LICENSE.txt
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# This file may also be used under the terms of a commercial license
+# if purchased from OpenC3, Inc.
+
+require 'securerandom'
+require 'benchmark'
+
+Benchmark.bm do |x|
+  data = SecureRandom.random_bytes(1024)
+  x.report("slice!") do
+    1000.times.each { data.slice!(0, 1) }
+  end
+  data = SecureRandom.random_bytes(1024)
+  x.report("assign") do
+    1000.times.each { data = data[1..-1] }
+  end
+  data = SecureRandom.random_bytes(1024)
+  x.report("replace") do
+    1000.times.each { data.replace(data[1..-1]) }
+  end
+end
+
+Benchmark.bm do |x|
+  data = SecureRandom.random_bytes(1024*1024)
+  x.report("slice!") do
+    1000.times.each { data.slice!(0, 1) }
+  end
+  data = SecureRandom.random_bytes(1024*1024)
+  x.report("assign") do
+    1000.times.each { data = data[1..-1] }
+  end
+  data = SecureRandom.random_bytes(1024*1024)
+  x.report("replace") do
+    1000.times.each { data.replace(data[1..-1]) }
+  end
+end

openc3/test/benchmarks/system_benchmark.rb

@@ -1,3 +1,21 @@
+# encoding: ascii-8bit
+
+# Copyright 2023 OpenC3, Inc.
+# All Rights Reserved.
+#
+# This program is free software; you can modify and/or redistribute it
+# under the terms of the GNU Affero General Public License
+# as published by the Free Software Foundation; version 3 with
+# attribution addendums as found in the LICENSE.txt
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# This file may also be used under the terms of a commercial license
+# if purchased from OpenC3, Inc.
+
 require 'openc3'
 require 'benchmark'
 ENV['OPENC3_NO_STORE'] = 'true'