Skip to content

Json2 (count): pre define buffer capacity to avoid memory copy and allocation - #20920

Merged
spytheman merged 37 commits into
vlang:masterfrom
enghitalo:json2/count
Mar 14, 2024
Merged

spytheman merged 37 commits into
vlang:masterfrom
enghitalo:json2/count

Conversation

@enghitalo

@enghitalo enghitalo commented Feb 28, 2024 •

Copy link
Copy Markdown
Contributor

Gains around 20% for now in vlib/v/tests/bench/bench_json_vs_json2.v;

benchmark to verify impact when using push_many

import time
import benchmark

const max_iterations = 100_000

fn main() {
	value := 'abcdefghijklmnopqrstuvdadkaodkpaiosdjaodijaodiajodia'

	mut buffer := []u8{}

	mut cap_inited_buf := []u8{cap: value.len * max_iterations}

	mut buffer2 := []u8{}

	mut cap_inited_buf2 := []u8{cap: value.len * max_iterations}

	mut b := benchmark.start()

	for i := 0; i < max_iterations; i++ {
		unsafe {buffer.push_many(value.str, value.len)}
	}

	b.measure("unsafe {buffer.push_many(value.str, value.len)}")

 	for i := 0; i < max_iterations; i++ {
		for letter in value {
			cap_inited_buf  << letter
		}
	}

	b.measure("cap_inited_buf  << letter")

	for i := 0; i < max_iterations; i++ {
		for letter in value {
			buffer2  << letter
		}
	}

	b.measure("buffer2  << letter")

	for i := 0; i < max_iterations; i++ {
		unsafe {cap_inited_buf2.push_many(value.str, value.len)}
	}

	b.measure("unsafe {cap_inited_buf2.push_many(value.str, value.len)}")
}
  • Aims: Use recursion, at compile time, without using workarounds.
  • option
  • sumtype
  • alias
  • pointer
  • enum
  • array

TODO

  • open a issue about chars_in_struct that should be working (This is breaking the CI)

@enghitalo
enghitalo marked this pull request as draft February 29, 2024 01:24
@enghitalo
enghitalo marked this pull request as ready for review March 9, 2024 11:59

@spytheman spytheman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new Count struct and its related API, does not need to be public.

The tests for it are very repetitive. The test file can also be an internal one (starting with module json2), thus avoiding exposing the helper Count struct.

@enghitalo

enghitalo commented Mar 11, 2024 •

Copy link
Copy Markdown
Contributor Author

The new Count struct and its related API, does not need to be public.

The tests for it are very repetitive. The test file can also be an internal one (starting with module json2), thus avoiding exposing the helper Count struct.

I thought this might be useful in cases like

mut count := json.Count{0}

count.count_chars(val)

mut request_buffer := 'POST / HTTP/1.1\r\nContent-Length: ${count.get_total()}\r\n\r\n'.bytes()

request_buffer.ensure_cap( request_buffer.len + count.get_total())

encoder.encode_value(val, mut request_buffer)!

What do you think?

@spytheman

Copy link
Copy Markdown
Contributor

I think, that we already have io reader/writer interfaces.

I also think that making something public, if there is a need for it in the future, justified by actual code and actual measurements in concrete situations, is easy.

In contrast, turning a public API to a private one, is much harder, costing time for deprecation, to minimize the breaking change. I also think, that the cost for both maintainers and users, of a non essential, inconvenient API is high.

@enghitalo

Copy link
Copy Markdown
Contributor Author

You rock! Thanks for the review.

@enghitalo
enghitalo requested a review from spytheman March 13, 2024 11:40
@spytheman
spytheman merged commit 49b7f9a into vlang:master Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants