Extract parsing int, string and byte into util functions

[sivachandran]

Branched from https://github.com/grafana/jfr-parser/pull/31

Extracts int, string and byte parsing implementation into parsing utilities to avoid redundant code.

The types are regenerated with parsing utilities.

[korniltsev]

Why do you think we need this? How do you make sure ParseVarLong & co are inlined ? (it is imporant because they are used in a hot path) The current approach is similar to what vtproto doing. https://github.com/grafana/pyroscope/blob/b16b27ac52a7e1c338234e8b792c1dbf78cc152a/api/gen/proto/go/google/v1/profile_vtproto.pb.go#L1509 The approach of no utils functions may or may not be better. I dont see any reason to change it if it is not better(faster). This PR seem to change nothing in terms of functionality, no changes in performance (hopefully - may need proofs/ tests/ benchmarks) and changes something in code readability( which we do not need as this code is generated) Am I missing something?

[sivachandran]

@korniltsev Thanks for having a look at my PR.

Why do you think we need this?

Not parsing char array string(case 4) here is how started the investigation. I then realised it is handled here. Instead of duplicating the char array string parsing implementation I decided to extract the parsing implementation into utility functions. In this way we can avoid such issues in the future. IMHO it also improves the readability and maintainability.

How do you make sure ParseVarLong & co are inlined ? (it is imporant because they are used in a hot path)

It was not obvious the code was duplicated for inlining purpose. So I didn't consider whether it would get inlined or not. After seeing your comment I analysed the generated executable with go tool objdump -s 'parser.types'. Except the string parsing utility function the other parsing utilities(int, long, byte) are all inlined by default.

The current approach is similar to what vtproto doing. https://github.com/grafana/pyroscope/blob/b16b27ac52a7e1c338234e8b792c1dbf78cc152a/api/gen/proto/go/google/v1/profile_vtproto.pb.go#L1509

I think my understanding of jfr-parser use cases is different from yours. I don't envision jfr-parser being used in highly time critical use scenario. I would prefer better readability/maintainability over few microseconds of cpu time.

If it is going to be used in highly time critical usage scenario then I would've implemented in Rust/Zig/C++ and called(CGO) from Go instead of implementing in pure Go. Despite the language choice I think it boils down to how much overhead we are incurring without inline. Do we have any benchmark on this?

changes something in code readability( which we do not need as this code is generated)

The PR improves the maintainability in addition to the readability. Now if we want to change int/long/string parsing then we don't need to touch multiple places. IMHO the readability improvement is not limited to the generated code but also the code that generates it(i.e., the template). The usage of function calls makes it easy to understand how the parsing works.

I understand if you are not convinced of using functions. Would you prefer closing this PR and submit new PR with just the fix?

[korniltsev]

Let's start with a functional change - char array string(case 4), this may be useful.

I'm not convinced we need to improve readability of generated code.

[korniltsev]

Except the string parsing utility

This may be important when parse symbol table.

[korniltsev]

I will close this PR for now.

PR's improving functionality or performance are welcome.