docs: fix install instructions and qualify ARM64 performance claims by membphis · Pull Request #58 · api7/lua-qjson

membphis · 2026-05-25T01:10:49Z

关联 Issue

Closes #57

做了什么

Replace luarocks install lua-qjson with luarocks make (package not yet published to luarocks.org)
Add note about future luarocks.org publication
Qualify ARM64 NEON/PMULL as correctness-tested, with benchmarks explicitly noted as x86_64 only
Add platform scope note to docs/benchmarks.md Environment section

Resolved Checklist Items

1.1 Verify local rockspec install path works
1.2 Update README Installing section
1.3 Add note about luarocks.org publication status
2.1 Decide approach — qualification only
2.2 Update README Status section — qualify ARM64 claim
2.3 Update docs/benchmarks.md Environment table — note x86_64 only
3.1 Execute README install instructions locally (rockspec validated via luarocks lint)
3.2 Read README end-to-end, confirm no remaining inconsistencies

如何验证

luarocks lint rockspec/lua-qjson-0.1.0-1.rockspec — rockspec valid, no errors
Manual read-through of README and docs/benchmarks.md — no stale references or inconsistencies

Breaking Changes

无

Summary by CodeRabbit

Release Notes

Documentation
- Updated installation instructions, specifying luarocks make as the primary installation method and clarifying the timeline for luarocks.org availability
- Added platform scope notes clarifying that published performance benchmarks cover x86_64 only, while ARM64 correctness is tested but performance metrics are not provided

- Replace luarocks install with luarocks make (package not yet published) - Add note about future luarocks.org publication - Qualify ARM64 NEON/PMULL as correctness-tested, benchmarks x86_64 only - Add platform scope note to docs/benchmarks.md Closes #57

coderabbitai · 2026-05-25T01:11:02Z

Warning

Review limit reached

@membphis, we couldn't start this review because you've used your available PR reviews for now.

Your plan includes 1 review of capacity. Refill in 11 minutes and 10 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b32c7f3f-82df-4a74-9127-3e8e818f45d8

📥 Commits

Reviewing files that changed from the base of the PR and between de38f67 and b959aec.

📒 Files selected for processing (3)

README.md
benches/arm_bench.lua
docs/benchmarks.md

📝 Walkthrough

Walkthrough

Documentation corrected to fix broken installation instructions and clarify ARM64 NEON support status. Installation now uses luarocks make with a note that direct luarocks install will be available after publication. ARM64 support is documented as correctness-tested only, with benchmarks noted as x86_64-specific.

Changes

Documentation accuracy updates

Layer / File(s)	Summary
Installation and platform scope clarification `README.md`, `docs/benchmarks.md`	Installation instructions changed from unavailable `luarocks install lua-qjson` to `luarocks make`, with a note that the luarocks.org package will be available after publication. README status section expanded to note ARM64 correctness testing via scanner cross-check but no performance benchmarks. Benchmarks documentation adds platform scope disclaimer that published results are x86_64-only.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

api7/lua-qjson#54: This PR's documentation updates directly reflect #54's changed benchmarking methodology and scenario set.
api7/lua-qjson#56: Both PRs update benchmark and README documentation; #56 reconciles benchmark numbers between README.md and docs/benchmarks.md.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main changes: fixing misleading install instructions and qualifying ARM64 performance claims in the documentation.
Linked Issues check	✅ Passed	All three objectives from issue `#57` are addressed: install instructions replaced with working `luarocks make` command with luarocks.org publication note, ARM64 NEON claim qualified as correctness-tested with x86_64-only benchmark note added to docs/benchmarks.md.
Out of Scope Changes check	✅ Passed	All changes are scoped to the issue requirements: only README.md and docs/benchmarks.md were modified for documentation clarity and accuracy, with no unrelated alterations.
E2e Test Quality Review	✅ Passed	PR contains only documentation changes (README.md install instructions and ARM64 qualification) with no tests or code modifications. E2E test quality review is not applicable.
Security Check	✅ Passed	PR modifies only documentation (README.md and docs/benchmarks.md). No code changes present; security check targets API Gateway platforms and is not applicable.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch docs/fix-install-and-arm64-docs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

Updates documentation to ensure installation instructions are accurate before luarocks.org publication, and to scope ARM64 performance claims to correctness-only (with benchmarks explicitly x86_64-only).

Changes:

Replace luarocks install lua-qjson with a local-install workflow and add a note about future luarocks.org availability.
Qualify ARM64 NEON/PMULL as correctness-tested (no published perf data) and clarify that published benchmark results are x86_64 only.
Add an explicit platform-scope note to the benchmarks documentation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
README.md	Updates install command and qualifies ARM64 performance claim / benchmark scope.
docs/benchmarks.md	Adds an explicit x86_64-only scope note for published benchmark data.

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

luarocks.org/modules/membphis/lua-qjson is live with 1,657 downloads. Keep the original install instruction; only the ARM64 qualification is needed.

- Add ARM64 parse+access throughput table (cjson comparison) to docs/benchmarks.md - Add arm_bench.lua reproduction script for macOS ARM64 - Label existing sections as x86_64 for clarity - Update platform scope note: ARM now has performance data - Add observation #9 contrasting ARM64 vs x86_64 speedup ratios - 1.8-13.8x over cjson on Apple M4; absolute qjson.parse throughput competitive with x86_64 Zen 2

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

 ## Status

-Initial implementation complete: scalar + AVX2/PCLMUL + ARM64 NEON/PMULL structural scanner (runtime-dispatched), root-path and cursor APIs, escape-decoded strings, integer/float/bool/typeof/len, FFI panic barrier, and a LuaJIT wrapper. Rust unit/integration tests and Lua busted tests run in CI. The benchmark harness compares against lua-cjson and lua-resty-simdjson.
+Initial implementation complete: scalar, AVX2/PCLMUL, and ARM64 NEON/PMULL structural scanners (runtime-dispatched); root-path and cursor APIs; escape-decoded strings; integer/float/bool/typeof/len accessors; FFI panic barrier; and a LuaJIT wrapper. Rust unit/integration tests and Lua busted tests run in CI. The benchmark harness compares against lua-cjson and lua-resty-simdjson. ARM64 NEON/PMULL is correctness-tested via the scanner cross-check suite; published benchmarks are x86_64 only.


 ## Status

-Initial implementation complete: scalar + AVX2/PCLMUL + ARM64 NEON/PMULL structural scanner (runtime-dispatched), root-path and cursor APIs, escape-decoded strings, integer/float/bool/typeof/len, FFI panic barrier, and a LuaJIT wrapper. Rust unit/integration tests and Lua busted tests run in CI. The benchmark harness compares against lua-cjson and lua-resty-simdjson.
+Initial implementation complete: scalar, AVX2/PCLMUL, and ARM64 NEON/PMULL structural scanners (runtime-dispatched); root-path and cursor APIs; escape-decoded strings; integer/float/bool/typeof/len accessors; FFI panic barrier; and a LuaJIT wrapper. Rust unit/integration tests and Lua busted tests run in CI. The benchmark harness compares against lua-cjson and lua-resty-simdjson. ARM64 NEON/PMULL is correctness-tested via the scanner cross-check suite; published benchmarks are x86_64 only.


+> ```sh
+> cargo build --release
+> LUA_PATH='./lua/?.lua;;' DYLD_LIBRARY_PATH=./target/release \
+>   luajit arm_bench.lua


+--     luajit arm_bench.lua
+


+B64_BLOCK = make_b64_block()
+B64_BLOCK_LEN = #B64_BLOCK


+--   DYLD_LIBRARY_PATH=./target/release LUA_CPATH='./vendor/lua-cjson/?.so;./target/release/lib?.so' \
+--     luajit arm_bench.lua
+
+package.cpath = "./vendor/lua-cjson/?.so;./target/release/lib?.so;" .. package.cpath


Copilot AI review requested due to automatic review settings May 25, 2026 01:10

Copilot started reviewing on behalf of membphis May 25, 2026 01:10 View session

Copilot AI reviewed May 25, 2026

View reviewed changes

Comment thread README.md

membphis added 2 commits May 25, 2026 09:53

docs: revert install section — luarocks install lua-qjson is published

423f463

luarocks.org/modules/membphis/lua-qjson is live with 1,657 downloads. Keep the original install instruction; only the ARM64 qualification is needed.

Copilot AI review requested due to automatic review settings May 25, 2026 01:59

Copilot started reviewing on behalf of membphis May 25, 2026 01:59 View session

Copilot AI reviewed May 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: fix install instructions and qualify ARM64 performance claims#58

docs: fix install instructions and qualify ARM64 performance claims#58
membphis wants to merge 3 commits into
mainfrom
docs/fix-install-and-arm64-docs

membphis commented May 25, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 25, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		B64_BLOCK = make_b64_block()
		B64_BLOCK_LEN = #B64_BLOCK

Conversation

membphis commented May 25, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

关联 Issue

做了什么

Resolved Checklist Items

如何验证

Breaking Changes

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

membphis commented May 25, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 25, 2026 •

edited

Loading