Re: [hash2][review] Early review (due to holidays)

From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2024-12-06 17:48:34

On 12/6/24 20:45, Peter Dimov via Boost wrote:
> Andrey Semashev wrote:
>> Yes, I understand, but the thing is I very rarely have to write `void f2( unsigned
>> char p[4] );` in the first place. Most of the time I get a variable amount of data
>> that I need to process, so I have either an iterator range or a pointer and size.
>> And if there are fixed-sized fragments of that data that I need to process,
>> pretty much always I have checked the entire size (or at least some outer size)
>> of the data beforehand, so no checks needed for those individual fragments.
>>
>> So, for example, if I have to parse an RTP packet, I would
>>
>> void on_rtp_packet(const uint8_t* packet, size_t size) {
>>   // RTP fixed header is 12 bytes long
>>   if (size < 12)
>>     throw std::invalid_argument("RTP packet too short");
>>
>>   // Parse 12-byte fixed header
>>   uint16_t seqn = read_be16(packet + 2);
>>   uint32_t timestamp = read_be32(packet + 4); }
> 
> read_be32 takes uint8_t const[4] here, because it has an implicit precondition
> that the argument has at least 4 valid bytes.
> 
> So
> 
> uint16_t read_be16( span<uint8_t const, 2> p );
> uint32_t read_be32( span<uint8_t const, 4> p );
> 
> void on_rtp_packet( span<uint8_t const> packet ) {
>   // RTP fixed header is 12 bytes long
>   if (packet.size() < 12)
>     throw std::invalid_argument("RTP packet too short");
> 
>   // Parse 12-byte fixed header
>   uint16_t seqn = read_be16(packet + 2);
>   uint32_t timestamp = read_be32(packet + 4);
> }
> 
> Since the optimizer sees that packet.size() >= 12, it
> can elide the checks against 2 and 4.

I don't trust the optimizer. Because it failed me more than once.

If you want your code fast, you better write it fast yourself and not
hope that the compiler does a good job for you.