Remove potentially too costly Packets::default()

[ryoqun]

Problem

I needed some cleanup while creating #14806 to ensure there is no similar issue.

The root problem is that current api is too easy to misuse. This follow-up pr is to fix the root cause and never reoccur similar issue in the future.

Summary of Changes

• Removed Packets::default(). As far as I read the code, this not need to be Default. By convention, Default::default() should be cheap but Pakcets violates that by over-allocating silently.
  • Also, implementing Default generally opens unbounded call-graph. So, I want to avoid it unless there is clear benefit/necessity to define Default.
• Then, identical functionality is renamed to Packets::for_test_and_bench to aptly indicate the purpose. New name is intentionally verbose and indicates this shouldn't be used in the production code.
  • Also checked that all current call-sites are tests and benches. (Yeah, I did this boring work once for all... lol)
• Demoted to_packets to #cfg(test), another source of potential cause of overallocation. It seems that production code doesn't actually need it as well.

This pr's cleanup motto is that let's make the ultimate call-site decide the allocation behavior by explicit call of to_packets_chunked.

This pr won't be back-ported to v1.4 intentionally. Maybe not worth do it.

Also, there is no functional change.

Fixes #

[codecov]

Codecov Report

Merging #14821 (5fb74f2) into master (015058e) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #14821   +/-   ##
=======================================
  Coverage    80.3%    80.3%           
=======================================
  Files         403      403           
  Lines      102747   102734   -13     
=======================================
- Hits        82551    82542    -9     
+ Misses      20196    20192    -4     

[sakridge]

How about just making Packets::default do a PinnedVec::default which just does a Vec::new(). Only reason to do with_capacity is performance, but if it's just used in the test context, then we don't really need that. I'm a bit mixed on the rename to test_or_bench.

I don't think the benchmarks do much allocating in the timing sections. In general it generates the packet vector up front, then runs the code on that.

[ryoqun]

How about just making Packets::default do a PinnedVec::default which just does a Vec::new(). Only reason to do with_capacity is performance, but if it's just used in the test context, then we don't really need that. I'm a bit mixed on the rename to test_or_bench.

I don't think the benchmarks do much allocating in the timing sections. In general it generates the packet vector up front, then runs the code on that.

@sakridge Thanks for review! That makes sense. I reverted most of changes. How does this pr look now?

[sakridge]

lgtm