-
-
Notifications
You must be signed in to change notification settings - Fork 427
Add extreme-scale performance tests with dynamic schema generation #2818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@koxudaxi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 8 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds extreme-scale performance testing infrastructure with three new fixtures generating large JSON schemas dynamically (2000+ models, 200 files, duplicate-name scenarios). It includes six new performance test functions and adjusts CI configuration by changing CodSpeed from instrumentation to simulation mode and omitting the performance test file from coverage tracking. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
|
📚 Docs Preview: https://pr-2818.datamodel-code-generator.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2818 +/- ##
==========================================
+ Coverage 98.72% 99.51% +0.79%
==========================================
Files 90 89 -1
Lines 14120 13924 -196
Branches 1661 1660 -1
==========================================
- Hits 13940 13857 -83
+ Misses 149 36 -113
Partials 31 31
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/main/test_performance.py (1)
659-688: Strengthen verification for massive file input tests.Both
test_perf_massive_files_inputandtest_perf_massive_files_single_outputonly verify that at least 1 Python file exists, but don't validate that the expected models were actually generated. The fixture creates 4000 models (200 files × 20 models), so the tests should verify a reasonable number of models were produced.Consider adding model counting similar to
test_perf_multiple_files_input(lines 218-222) to ensure the generator successfully processed the massive input.🔎 Proposed enhancement
@pytest.mark.perf def test_perf_massive_files_input(tmp_path: Path, massive_files_input: Path) -> None: """Performance test: Process 200 separate schema files (4000 models total). Tests directory input handling with a very large number of files. """ output_dir = tmp_path / "output" generate( input_=massive_files_input, input_file_type=InputFileType.JsonSchema, output=output_dir, ) assert output_dir.exists() py_files = list(output_dir.glob("**/*.py")) assert len(py_files) >= 1 + # Verify a substantial number of models were generated + total_models = 0 + for py_file in py_files: + content = py_file.read_text() + total_models += content.count("class Module") + assert total_models >= 1000 # At least 25% of the 4000 modelsApply similar enhancement to
test_perf_massive_files_single_output.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/codspeed.yamlpyproject.tomltests/main/test_performance.py
🧰 Additional context used
🧬 Code graph analysis (1)
tests/main/test_performance.py (2)
src/datamodel_code_generator/__init__.py (1)
generate(450-1016)src/datamodel_code_generator/enums.py (2)
InputFileType(35-45)DataModelType(48-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: benchmarks
- GitHub Check: 3.10 on macOS
- GitHub Check: py312-pydantic1 on Ubuntu
- GitHub Check: py312-isort5 on Ubuntu
- GitHub Check: py312-isort6 on Ubuntu
- GitHub Check: py312-black23 on Ubuntu
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.11 on Ubuntu
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.13 on Ubuntu
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
pyproject.toml (1)
246-246: LGTM! Appropriate exclusion of performance tests from coverage.Excluding performance tests from coverage tracking is the correct approach, as these tests focus on timing and scalability rather than code path coverage.
.github/workflows/codspeed.yaml (1)
35-35: Verify the mode change from "instrumentation" to "simulation" is intentional.The CodSpeed mode has been changed from "instrumentation" to "simulation." This is a significant change that affects how performance is measured:
- Instrumentation mode: Profiles actual execution with precise measurements but adds overhead
- Simulation mode: Uses heuristics/sampling, faster but potentially less accurate
Given that this PR adds extreme-scale tests (2000+ models, 200 files), simulation mode may be necessary to avoid excessive overhead. However, please confirm:
- Is this change intentional for the new extreme-scale tests?
- Are there known compatibility or performance issues with instrumentation mode for these tests?
- Does simulation mode provide adequate metrics for your performance tracking needs?
If the mode change is specifically for the extreme tests, consider whether separate workflows (one for standard benchmarks in instrumentation mode, one for extreme tests in simulation mode) would provide better measurement fidelity.
CodSpeed Performance ReportMerging #2818 will create unknown performance changesComparing
|
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR only adds new performance tests and modifies CI/CD configuration. It does not change any production code, APIs, CLI options, templates, code generation logic, default behaviors, Python version support, or error handling. All changes are isolated to test infrastructure: new test functions in tests/main/test_performance.py, coverage configuration in pyproject.toml, and CodSpeed workflow settings in .github/workflows/codspeed.yaml. This analysis was performed by Claude Code Action |
Summary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.