3 ms·

To bring this discussion back to earth, the Go code in question is: https://github.com/VictoriaMetrics/easyproto/blob/master/reader.go https://github.com/Victor

by kjksf 3y ago

To bring this discussion back to earth, the Go code in question is: https://github.com/VictoriaMetrics/easyproto/blob/master/reader.go https://github.com/VictoriaMetrics/easyproto/blob/master/rea... I don't see why did you label Aliaksandr Valialkin, the author, an "expert". I mean, he's no dummy but what exactly makes him an expert on optimizing Go code? As someone who also writes Go, I don't see any "hyper optimizations" in the code. It just decodes the bytes of Protocol Buffer using straightforward code that I would expect a competent developer to write. It really is just: read bytes from memory ([]byte) and interpret them according to PB spec. There's only one trick there: unsafeBytesToString() that does no-allocation conversion of []byte to string. This is unsafe in general but safe in their specific case. And I've seen this trick before so it's not some secret, expert-only knowledge. Most comments here are like bad LLMs: hallucinating opinions without bothering to spent even few minutes acquiring the data to base those opinions on.

sgift 3y ago
> I don't see why did you label Aliaksandr Valialkin, the author, an "expert". I mean, he's no dummy but what exactly makes him an expert on optimizing Go code? https://victoriametrics.com/team/ https://victoriametrics.com/team/ - Let's see. Author of multiple performance-optimized libraries with a masters degree in computer software engineering and a background in highly scalable systems (as needed for adtech). Sounds pretty much like an expert for optimizing code to me. > And I've seen this trick before so it's not some secret, expert-only knowledge. So, if you know it it's not export knowledge or what's your argument here?
kasey_junk 3y ago
He’s probably most well known in the golang community for fasthttp which is a widely used and highly optimized golang replacement for the golang stdlib. I’m a long term golang developer and I think calling him a golang optimization “expert” is fair. That said, I agree with your assessment about this particular code. It’s fairly straightforward idiomatic go.
sapiogram 3y ago
> I don't see why did you label Aliaksandr Valialkin, the author, an "expert". I mean, he's no dummy but what exactly makes him an expert on optimizing Go code? I was trying to convey the meaning of "far more experienced than the blog post authors", but without having to insult the authors. It's a good writeup after all, and I'm glad they took the time. We must have some different interpretations of what "optimized" means. This is the very first piece of code in the file you linked: func (fc *FieldContext) NextField(src []byte) ([]byte, error) { if len(src) >= 2 { n := uint16(src[0])<<8 | uint16(src[1]) if (n&0x8080 == 0) && (n&0x0700 == (uint16(wireTypeLen) << 8)) { // Fast path - read message with the length smaller than 0x80 bytes. msgLen := int(n & 0xff) src = src[2:] if len(src) < msgLen { return src, fmt.Errorf("cannot read field for from %d bytes; need at least %d bytes", len(src), msgLen) } fc.FieldNum = uint32(n >> (8 + 3)) fc.wireType = wireTypeLen fc.data = src[:msgLen] src = src[msgLen:] return src, nil } } // ... function continues beyond this point As far as I can tell, this entire codepath exists solely as an optimization. I spent many years working on a chess engine for fun, so I'm pretty well versed in bit twiddling, but I'm seriously struggling with this. Like, is it doing `(n&0x8080 == 0)` to check to whether length is less than 0x80? Is that even correct? I think "hyper optimized" is a completely fair characterization. But we clearly work in different industries.
- tedunangst 3y ago
  I'm not sure the presence of bit unpacking code in a decoder for a bit packed protocol is sufficient to call it hyper optimized. That seems like the nature of the problem.
- ndriscoll 3y ago
  Protobuf uses a bunch of variable length encodings. Here it's decoding a TLV format, but the length is itself a variable length integer (and seems to be a kind of tag-value encoding?) where you basically get 7 bits per byte telling you the value, and the leftmost bit tells you whether there's another byte. So if you mask with 0x8080 and get zero, then it was a 1 byte (7 bit) integer. If 0x8080 is not set, then the tag-value record is 2 bytes. Left byte has tag. Right is value. Then they're masking with 0x0700 to get the type of record, which should be LEN. So if it's a single byte LEN record, they can take that single byte as the length (they mask with 0x00ff, but really it's 0x007f. They already know the 0x80 bit is zero, and the value is contained in the least significant 7 bits). Otherwise they have to do some fiddly logic to decode the variable length integer to figure out the length (length here being the L in TLV).

by kjksf 3y ago

sgift 3y ago

> I don't see why did you label Aliaksandr Valialkin, the author, an "expert". I mean, he's no dummy but what exactly makes him an expert on optimizing Go code? https://victoriametrics.com/team/ https://victoriametrics.com/team/ - Let's see. Author of multiple performance-optimized libraries with a masters degree in computer software engineering and a background in highly scalable systems (as needed for adtech). Sounds pretty much like an expert for optimizing code to me. > And I've seen this trick before so it's not some secret, expert-only knowledge. So, if you know it it's not export knowledge or what's your argument here?

kasey_junk 3y ago

He’s probably most well known in the golang community for fasthttp which is a widely used and highly optimized golang replacement for the golang stdlib. I’m a long term golang developer and I think calling him a golang optimization “expert” is fair. That said, I agree with your assessment about this particular code. It’s fairly straightforward idiomatic go.

sapiogram 3y ago

> I don't see why did you label Aliaksandr Valialkin, the author, an "expert". I mean, he's no dummy but what exactly makes him an expert on optimizing Go code? I was trying to convey the meaning of "far more experienced than the blog post authors", but without having to insult the authors. It's a good writeup after all, and I'm glad they took the time. We must have some different interpretations of what "optimized" means. This is the very first piece of code in the file you linked: func (fc *FieldContext) NextField(src []byte) ([]byte, error) { if len(src) >= 2 { n := uint16(src[0])<<8 | uint16(src[1]) if (n&0x8080 == 0) && (n&0x0700 == (uint16(wireTypeLen) << 8)) { // Fast path - read message with the length smaller than 0x80 bytes. msgLen := int(n & 0xff) src = src[2:] if len(src) < msgLen { return src, fmt.Errorf("cannot read field for from %d bytes; need at least %d bytes", len(src), msgLen) } fc.FieldNum = uint32(n >> (8 + 3)) fc.wireType = wireTypeLen fc.data = src[:msgLen] src = src[msgLen:] return src, nil } } // ... function continues beyond this point As far as I can tell, this entire codepath exists solely as an optimization. I spent many years working on a chess engine for fun, so I'm pretty well versed in bit twiddling, but I'm seriously struggling with this. Like, is it doing `(n&0x8080 == 0)` to check to whether length is less than 0x80? Is that even correct? I think "hyper optimized" is a completely fair characterization. But we clearly work in different industries.

tedunangst 3y ago
I'm not sure the presence of bit unpacking code in a decoder for a bit packed protocol is sufficient to call it hyper optimized. That seems like the nature of the problem.
ndriscoll 3y ago
Protobuf uses a bunch of variable length encodings. Here it's decoding a TLV format, but the length is itself a variable length integer (and seems to be a kind of tag-value encoding?) where you basically get 7 bits per byte telling you the value, and the leftmost bit tells you whether there's another byte. So if you mask with 0x8080 and get zero, then it was a 1 byte (7 bit) integer. If 0x8080 is not set, then the tag-value record is 2 bytes. Left byte has tag. Right is value. Then they're masking with 0x0700 to get the type of record, which should be LEN. So if it's a single byte LEN record, they can take that single byte as the length (they mask with 0x00ff, but really it's 0x007f. They already know the 0x80 bit is zero, and the value is contained in the least significant 7 bits). Otherwise they have to do some fiddly logic to decode the variable length integer to figure out the length (length here being the L in TLV).