JSONEncoder: fixed _asDirectArrayEncodable function #1593
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In my recent PR, I optimized
JSONDecoder/JSONEncoder. However, I introduced a small mistake in the_asDirectArrayEncodablefunction:Instead of using the value parameter from the function arguments, I mistakenly referenced the array property from the
__JSONEncoderclass.Since
__JSONEncoder.arrayis of typeJSONFuture.RefArray, all type equality checks (e.g. against[Int]) failed.As a result, the intended optimization did not take effect.
This issue slipped past benchmarks because there are currently no tests for decoding large
[Int](or similar) arrays.To address this, I’ve added an additional benchmark that encodes 100k Ints, which highlights the performance improvements.
array-encodeToJSON metrics
Time (total CPU): results within specified thresholds, fold down for details.
Throughput (# / s): results within specified thresholds, fold down for details.
I’m happy to either include the new benchmark in this PR or submit it separately, depending on the maintainers’ preference. We can significantly reduce risks of introducing encoding large arrays performance degradations with separate benchmark.