friendlymatthew commented on code in PR #7878: URL: https://github.com/apache/arrow-rs/pull/7878#discussion_r2196386599
########## parquet-variant-json/benches/variant_validation.rs: ########## @@ -0,0 +1,137 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +extern crate parquet_variant; Review Comment: > I found these benchmarks somewhat confusing -- they are mostly testing json --> Variant conversion I think. > > While those are useful benchmarks, they probably shouldn't be called variant_validation Hi, the following benchmarks use [iter_batched](https://docs.rs/criterion/latest/criterion/struct.Bencher.html#method.iter_batched) which doesn't include the startup time (in this case, building up the variant). The benchmark runs should only involve the validation code. Reading this [issue](https://github.com/bheisler/criterion.rs/issues/475), it seems like `iter_batched_ref` is a better function to use. As for the `json_to_variant`, I could've built up the variant manually via the builder, but found the json example quite convenient. Since we moved `json_to_variant` to a separate crate, I had no choice but to move the validation benchmarks here. But I agree, we should move this back to `parquet-variant`. It is a bit unfortunate, but we can manually build up the objects. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
