Skip to content

Conversation

@stormslowly
Copy link
Contributor

@stormslowly stormslowly commented Oct 28, 2025

Adapt simd_json for faster(20~30% improvement) JSON parse and use simd_json value for internal JSON representation.

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 28, 2025

CodSpeed Performance Report

Merging #117 will improve performances by 32.29%

Comparing perf/simd_borrowed (99cb865) with main (0c268e1)

Summary

⚡ 2 improvements
✅ 3 untouched

Benchmarks breakdown

Benchmark BASE HEAD Change
resolver[multi-thread] 129.1 ms 97.6 ms +32.29%
resolver[single-thread] 112.1 ms 91.1 ms +23.06%

@stormslowly stormslowly marked this pull request as ready for review November 6, 2025 08:43
Copilot AI review requested due to automatic review settings November 6, 2025 08:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the JSON parsing implementation from serde_json to simd-json for improved performance when parsing package.json files. The changes introduce strongly-typed enums for the type and sideEffects fields and update the file system API to work with Vec<u8> instead of String.

  • Replaces serde_json with simd-json for JSON parsing with borrowed values
  • Introduces ModuleType and SideEffects enums for type-safe field representation
  • Updates file system reads to use bytes instead of strings

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
Cargo.toml Adds simd-json and self_cell dependencies
Cargo.lock Updates dependency tree with new and updated packages
deny.toml Adds Zlib license to allowed list
src/package_json/mod.rs Adds ModuleType and SideEffects enum definitions
src/package_json/simd.rs Migrates from serde_json to simd-json with borrowed values
src/lib.rs Updates JSON type references and pattern matching
src/cache.rs Changes file read from string to bytes
napi/src/lib.rs Simplifies module type extraction using new enum
tests/integration_test.rs Updates tests to use new typed fields
tests/package.json Fixes invalid JSON value for sideEffects field
src/tests/imports_field.rs Updates test helpers to use simd-json
src/tests/exports_field.rs Updates test helpers to use simd-json
Comments suppressed due to low confidence (2)

src/package_json/simd.rs:105

  • The method chain is incorrect. get returns Option<&JSONValue>, so .as_str() is being called on the Option instead of the JSONValue. This should be .and_then(|v| v.as_str()) to first extract the value from the Option.
    src/package_json/simd.rs:316
  • Column positions should be counted by character positions, not byte positions. For multi-byte UTF-8 characters, this will produce incorrect column numbers. Change to col += 1 to count characters instead of bytes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@stormslowly stormslowly requested a review from hardfist November 6, 2025 08:45
@stormslowly stormslowly changed the title feat: wip borrowed value perf: use simd json for json parse Nov 6, 2025
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@stormslowly stormslowly requested a review from quininer November 6, 2025 08:55
Copy link

@quininer quininer left a comment

Choose a reason for hiding this comment

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

LGTM

@stormslowly stormslowly merged commit b8f4b11 into main Nov 7, 2025
22 checks passed
@stormslowly stormslowly deleted the perf/simd_borrowed branch November 7, 2025 03:12
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.

4 participants