jiter

jiter copied to clipboard

#90 Add basic comment support

CodSpeed Performance Report

Merging #91 will degrade performances by 24.96%

_{Comparing 1aam2am1:main (830309a) with main (75699eb)}

Summary

❌ 12 regressions ✅ 61 untouched benchmarks

:warning: Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	main	1aam2am1:main	Change
❌	bigints_array_jiter_skip	500.4 µs	556.9 µs	-10.16%
❌	medium_response_jiter_skip	73.5 µs	83.7 µs	-12.19%
❌	pass1_jiter_skip	53.2 µs	60.6 µs	-12.17%
❌	pass2_jiter_iter	19.4 µs	21.6 µs	-10.28%
❌	pass2_jiter_skip	5.5 µs	7.4 µs	-24.96%
❌	short_numbers_jiter_skip	332.7 µs	374.5 µs	-11.17%
❌	string_array_jiter_iter	43.3 µs	50.9 µs	-14.82%
❌	string_array_jiter_skip	37.2 µs	42.9 µs	-13.29%
❌	true_array_jiter_iter	24 µs	27.4 µs	-12.5%
❌	true_array_jiter_skip	22.9 µs	27.6 µs	-17.05%
❌	true_object_jiter_skip	56.2 µs	66.7 µs	-15.67%
❌	python_parse_true_array	49.8 µs	56 µs	-11.11%

Codecov Report

Attention: Patch coverage is 0% with 21 lines in your changes are missing coverage. Please review.

Files	Patch %	Lines
crates/jiter/src/parse.rs	0.00%	21 Missing :warning:

:loudspeaker: Thoughts on this report? Let us know!

The use case of supporting comments is 👍 from me, but I would like us to find a way to do this without costing performance.

I suspect that the introduction of a large match arm inside the eat_whitespace function has led to the performance regression.

The fuzz jobs are failing because serde doesn't support comments.

The most practical way to solve both of these may be to make comment support optional and find a way to refactor to avoid the perf hit when unused.

Hi. This is great idea, to support it as a option, where you could toggle it. For example from pydantic python.

But as I don't know rust very much and don't know exactly how this project here is done, I can't write a updated code. I think we would need some template programming (If something like this exists in rust). And simply select engine with comments or without depending on option. So that we don't have extra branch there. As any if added there would have speed penalty.

Yep, Rust has generics which are the closest equivalent it has to C++ templates.

This isn't high priority for me to pick up right now, but anyone with interest could take this on (or maybe I can add in the future).

closing until someone wants to take this on.