perf(api): initialize StringBuilder with specified capacities for performance#1087
perf(api): initialize StringBuilder with specified capacities for performance#1087balazs-szucs wants to merge 5 commits intogrimmory-tools:developfrom
Conversation
…acities for performance
…of reallocating objects in CoverImageGenerator
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ 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). (4)
🧰 Additional context used📓 Path-based instructions (2)backend/src/**/*.java📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
WalkthroughThis PR updates multiple backend places to preallocate or reuse StringBuilder instances (specific capacities) and replaces a manual MD5 hex loop with Java's HexFormat; no public APIs or control flow semantics are changed. (50 words) ChangesStringBuilder preallocation & small util change
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Review rate limit: 6/8 reviews remaining, refill in 14 minutes and 54 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/main/java/org/booklore/util/Md5Util.java`:
- Around line 19-22: Replace the manual per-byte String.format loop in Md5Util
(the code that builds a hex string from the digest using StringBuilder and
String.format inside the for-each) with Java's HexFormat: import
java.util.HexFormat and return HexFormat.of().formatHex(digest) (this yields the
same lowercase hex output but is far faster and simpler than the per-byte
String.format loop).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 51e5b54b-7e71-47af-b38c-a441f5f20496
📒 Files selected for processing (12)
backend/src/main/java/org/booklore/app/service/AppBookService.javabackend/src/main/java/org/booklore/controller/OidcAuthController.javabackend/src/main/java/org/booklore/service/InstallationService.javabackend/src/main/java/org/booklore/service/bookdrop/FilenamePatternExtractor.javabackend/src/main/java/org/booklore/service/metadata/CoverImageGenerator.javabackend/src/main/java/org/booklore/service/metadata/extractor/AudiobookMetadataExtractor.javabackend/src/main/java/org/booklore/service/metadata/extractor/Fb2MetadataExtractor.javabackend/src/main/java/org/booklore/service/metadata/parser/GoogleParser.javabackend/src/main/java/org/booklore/service/metadata/writer/CbxMetadataWriter.javabackend/src/main/java/org/booklore/service/metadata/writer/PdfMetadataWriter.javabackend/src/main/java/org/booklore/service/reader/AudioFileUtilityService.javabackend/src/main/java/org/booklore/util/Md5Util.java
📜 Review details
⏰ 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). (5)
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Frontend Lint Threshold Check
🧰 Additional context used
📓 Path-based instructions (5)
backend/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
backend/src/**/*.java: Use 4-space indentation and match surrounding Java style in backend code
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection in backend code
Use MapStruct for entity/DTO mapping in backend code
Keep JPA entities on the *Entity suffix in backend code
Files:
backend/src/main/java/org/booklore/util/Md5Util.javabackend/src/main/java/org/booklore/service/InstallationService.javabackend/src/main/java/org/booklore/app/service/AppBookService.javabackend/src/main/java/org/booklore/controller/OidcAuthController.javabackend/src/main/java/org/booklore/service/metadata/extractor/Fb2MetadataExtractor.javabackend/src/main/java/org/booklore/service/reader/AudioFileUtilityService.javabackend/src/main/java/org/booklore/service/metadata/parser/GoogleParser.javabackend/src/main/java/org/booklore/service/metadata/extractor/AudiobookMetadataExtractor.javabackend/src/main/java/org/booklore/service/bookdrop/FilenamePatternExtractor.javabackend/src/main/java/org/booklore/service/metadata/writer/PdfMetadataWriter.javabackend/src/main/java/org/booklore/service/metadata/CoverImageGenerator.javabackend/src/main/java/org/booklore/service/metadata/writer/CbxMetadataWriter.java
**/*
⚙️ CodeRabbit configuration file
**/*: This project is being developed using current and future-facing technologies:
- Java 25 with --enable-preview (preview features are INTENTIONAL and encouraged)
- Spring Boot 4 (latest major version, check APIs accordingly)
- Jackson 3 (new package: tools.jackson.* instead of com.fasterxml.jackson.*)
- Hibernate 7.3.x (Jakarta Persistence 3.2, new APIs; avoid deprecated Hibernate 5/6 patterns)
- Angular 21 (signals-based reactivity, no NgModules unless legacy)
Grimmory Internal Tools
- epub4j and pdfium4j are our own internal tools developed by the Grimmory team.
- Always verify behavior and API changes against the upstream repositories:
- If you encounter issues with these libraries, check if a fix exists in the upstream grimmory-tools organization.
Metadata Standards and Compliance
- For all metadata writing and parsing logic, double-check against Dublin Core and ANSI standards to ensure perfect official compliance.
- We strictly follow the widespread and official XML-compliant methods for EPUB2, EPUB3, CBX, and PDF formats.
General Java and Spring rules
- ALWAYS prefer modern, idiomatic Java 25 constructs over legacy patterns.
- Preview features (--enable-preview) are enabled and intentional; do NOT flag them as risky unless there is a concrete runtime issue.
- Prefer: records, sealed classes/interfaces, pattern matching (switch expressions, instanceof), structured concurrency (StructuredTaskScope), scoped values, string templates, unnamed patterns/variables.
- Prefer virtual threads (Thread.ofVirtual(), Executors.newVirtualThreadPerTaskExecutor()) over platform threads for I/O-bound work.
- Prefer the new Sequenced Collections API (SequencedCollection, SequencedMap) where applicable.
- Prefer
varfor local variables when the type is obvious from context.- Use stream().toList() instead of stream().collect(Collectors.toList()) for imm...
Files:
backend/src/main/java/org/booklore/util/Md5Util.javabackend/src/main/java/org/booklore/service/InstallationService.javabackend/src/main/java/org/booklore/app/service/AppBookService.javabackend/src/main/java/org/booklore/controller/OidcAuthController.javabackend/src/main/java/org/booklore/service/metadata/extractor/Fb2MetadataExtractor.javabackend/src/main/java/org/booklore/service/reader/AudioFileUtilityService.javabackend/src/main/java/org/booklore/service/metadata/parser/GoogleParser.javabackend/src/main/java/org/booklore/service/metadata/extractor/AudiobookMetadataExtractor.javabackend/src/main/java/org/booklore/service/bookdrop/FilenamePatternExtractor.javabackend/src/main/java/org/booklore/service/metadata/writer/PdfMetadataWriter.javabackend/src/main/java/org/booklore/service/metadata/CoverImageGenerator.javabackend/src/main/java/org/booklore/service/metadata/writer/CbxMetadataWriter.java
**/service/**/*.java
⚙️ CodeRabbit configuration file
**/service/**/*.java: Spring Framework 7 service layer review:
- Flag missing
@Transactionalon methods that perform multiple writes.- Prefer constructor injection over
@Autowiredfield injection. Use@AllArgsConstructor.- Use ApiError enum for throwing exceptions.
- Flag checked exceptions swallowed silently; must log or rethrow.
- Flag Thread.sleep(); prefer Duration-based overloads or ScheduledExecutorService.
- Prefer virtual threads (Thread.ofVirtual()) for I/O-bound operations.
- Flag mutable shared state in singleton beans.
Files:
backend/src/main/java/org/booklore/service/InstallationService.javabackend/src/main/java/org/booklore/app/service/AppBookService.javabackend/src/main/java/org/booklore/service/metadata/extractor/Fb2MetadataExtractor.javabackend/src/main/java/org/booklore/service/reader/AudioFileUtilityService.javabackend/src/main/java/org/booklore/service/metadata/parser/GoogleParser.javabackend/src/main/java/org/booklore/service/metadata/extractor/AudiobookMetadataExtractor.javabackend/src/main/java/org/booklore/service/bookdrop/FilenamePatternExtractor.javabackend/src/main/java/org/booklore/service/metadata/writer/PdfMetadataWriter.javabackend/src/main/java/org/booklore/service/metadata/CoverImageGenerator.javabackend/src/main/java/org/booklore/service/metadata/writer/CbxMetadataWriter.java
**/controller/**/*.java
⚙️ CodeRabbit configuration file
**/controller/**/*.java: Spring Framework 7 REST controller review:
- Prefer
@RestController+@RequestMappingwith ResponseEntity.- Flag missing input validation (
@Valid/@Validatedon request body/params).- Prefer ProblemDetail (RFC 9457) for error responses.
- Flag
@RequestMappingwithout explicit HTTP method constraint.- Check for missing authorization checks on sensitive endpoints.
- Flag any logging of request body that may contain PII or credentials.
Files:
backend/src/main/java/org/booklore/controller/OidcAuthController.java
**/metadata/**/*.java
⚙️ CodeRabbit configuration file
**/metadata/**/*.java: Metadata and Sidecar review:
- Ensure perfect official compliance with Dublin Core and ANSI standards.
- Strictly follow official XML-compliant methods for EPUB2, EPUB3, CBX, and PDF formats.
- Sidecar files must be written in a way that is compatible with widespread e-reader standards.
- Avoid proprietary or non-standard XML tags unless absolutely necessary for internal tracking.
Files:
backend/src/main/java/org/booklore/service/metadata/extractor/Fb2MetadataExtractor.javabackend/src/main/java/org/booklore/service/metadata/parser/GoogleParser.javabackend/src/main/java/org/booklore/service/metadata/extractor/AudiobookMetadataExtractor.javabackend/src/main/java/org/booklore/service/metadata/writer/PdfMetadataWriter.javabackend/src/main/java/org/booklore/service/metadata/CoverImageGenerator.javabackend/src/main/java/org/booklore/service/metadata/writer/CbxMetadataWriter.java
🧠 Learnings (2)
📚 Learning: 2026-04-10T08:15:37.436Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 449
File: booklore-api/src/main/java/org/booklore/service/book/BookDownloadService.java:139-145
Timestamp: 2026-04-10T08:15:37.436Z
Learning: When using Spring `ContentDisposition.builder(...).filename(name, StandardCharsets.UTF_8).build()` (i.e., explicitly providing UTF-8), the resulting header value should include both the quoted `filename="=?UTF-8?..."` and the RFC 5987 `filename*=` parameters. In this case, any extra ASCII fallback computation (e.g., deriving an ASCII `fallbackFilename` via `NON_ASCII_PATTERN` and calling `.filename(fallbackFilename)`) is likely redundant—prefer calling only `.filename(fallbackName?, StandardCharsets.UTF_8)` as appropriate and let Spring handle the UTF-8 header parameters. Verify by comparing the emitted header for `filename` and `filename*` before deciding to keep an ASCII fallback.
Applied to files:
backend/src/main/java/org/booklore/util/Md5Util.javabackend/src/main/java/org/booklore/service/InstallationService.javabackend/src/main/java/org/booklore/app/service/AppBookService.javabackend/src/main/java/org/booklore/controller/OidcAuthController.javabackend/src/main/java/org/booklore/service/metadata/extractor/Fb2MetadataExtractor.javabackend/src/main/java/org/booklore/service/reader/AudioFileUtilityService.javabackend/src/main/java/org/booklore/service/metadata/parser/GoogleParser.javabackend/src/main/java/org/booklore/service/metadata/extractor/AudiobookMetadataExtractor.javabackend/src/main/java/org/booklore/service/bookdrop/FilenamePatternExtractor.javabackend/src/main/java/org/booklore/service/metadata/writer/PdfMetadataWriter.javabackend/src/main/java/org/booklore/service/metadata/CoverImageGenerator.javabackend/src/main/java/org/booklore/service/metadata/writer/CbxMetadataWriter.java
📚 Learning: 2026-04-14T12:43:08.698Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:43:08.698Z
Learning: For this codebase (booklore-api), target Java 25 with `--enable-preview`, so `_` is intentionally used as an unnamed/ignored variable (e.g., lambda parameter or pattern variable) per Java’s preview feature JEP 456. Do not flag `_` in those contexts as an invalid/reserved identifier; only flag it if it’s used in a non-supported position (e.g., where an unnamed variable is not applicable for the Java preview rules).
Applied to files:
backend/src/main/java/org/booklore/util/Md5Util.javabackend/src/main/java/org/booklore/service/InstallationService.javabackend/src/main/java/org/booklore/app/service/AppBookService.javabackend/src/main/java/org/booklore/controller/OidcAuthController.javabackend/src/main/java/org/booklore/service/metadata/extractor/Fb2MetadataExtractor.javabackend/src/main/java/org/booklore/service/reader/AudioFileUtilityService.javabackend/src/main/java/org/booklore/service/metadata/parser/GoogleParser.javabackend/src/main/java/org/booklore/service/metadata/extractor/AudiobookMetadataExtractor.javabackend/src/main/java/org/booklore/service/bookdrop/FilenamePatternExtractor.javabackend/src/main/java/org/booklore/service/metadata/writer/PdfMetadataWriter.javabackend/src/main/java/org/booklore/service/metadata/CoverImageGenerator.javabackend/src/main/java/org/booklore/service/metadata/writer/CbxMetadataWriter.java
🔇 Additional comments (15)
backend/src/main/java/org/booklore/service/metadata/parser/GoogleParser.java (1)
516-516: Looks good — preallocation is appropriate here.This change is aligned with the method’s bounded output length and should reduce avoidable buffer growth.
backend/src/main/java/org/booklore/service/bookdrop/FilenamePatternExtractor.java (2)
257-257: Good preallocation in regex builder path.Initializing
StringBuilderwith an explicit capacity here is a safe micro-optimization for this hot string-construction path and keeps behavior unchanged.
330-330: Nice improvement for date-format regex assembly.This capacity hint is appropriate for the method’s append pattern and is consistent with the PR’s performance goal without changing logic.
backend/src/main/java/org/booklore/service/metadata/writer/CbxMetadataWriter.java (1)
249-249: StringBuilder preallocation is appropriate here.This is a safe micro-optimization and keeps behavior unchanged.
backend/src/main/java/org/booklore/app/service/AppBookService.java (1)
1171-1171: Good low-risk optimization in JPQL scope assembly.Pre-sizing the builder is fine and does not alter query clause behavior.
backend/src/main/java/org/booklore/service/metadata/extractor/Fb2MetadataExtractor.java (1)
294-294: Looks good — capacity hint fits this text-accumulation path.No functional differences introduced in annotation text extraction.
backend/src/main/java/org/booklore/service/metadata/extractor/AudiobookMetadataExtractor.java (1)
360-360: Approved: preallocation is appropriate for ffprobe JSON capture.This improves allocation behavior without changing chapter extraction logic.
backend/src/main/java/org/booklore/controller/OidcAuthController.java (1)
101-101: Nice micro-optimization in redirect fragment construction.Behavior remains the same and capacity hint is reasonable for this payload size.
backend/src/main/java/org/booklore/service/reader/AudioFileUtilityService.java (1)
105-105: Approved: capacity hints are appropriate for numeric chunks.This keeps the natural sort logic intact while reducing transient allocations.
Also applies to: 109-109
backend/src/main/java/org/booklore/service/InstallationService.java (1)
83-83: Great fit: exact-capacity allocation for SHA-256 hex output.
64is the right target size for this encoding path.backend/src/main/java/org/booklore/service/metadata/writer/PdfMetadataWriter.java (2)
129-129: Looks good — preallocating keywords buffer is safe and useful.No functional change to PDF metadata writing behavior.
329-329: Approved: bag XML accumulator preallocation is reasonable.This keeps XMP injection semantics unchanged while reducing growth reallocations.
backend/src/main/java/org/booklore/service/metadata/CoverImageGenerator.java (3)
416-416: LGTM! Good StringBuilder capacity pre-allocation.The 128-character initial capacity is appropriate for formatting author names (sanitized to MAX_AUTHOR_LEN=100) with newline separators, reducing potential reallocation overhead.
645-679: LGTM! Excellent StringBuilder optimization with capacity hint and reuse pattern.The combination of pre-allocating capacity (32 chars for word accumulation) and reusing the same
StringBuilderinstance viasetLength(0)instead of creating new instances is a best practice that reduces both allocation overhead and GC pressure during text wrapping.
681-695: LGTM! Consistent StringBuilder optimization pattern applied.The same effective pattern from
wrapText(capacity pre-allocation + reuse viasetLength(0)) is correctly applied here for breaking long words.
Description
This pull request optimizes memory usage and performance by pre-allocating StringBuilder instances with an initial capacity throughout various parts of the codebase. By specifying an appropriate initial size for each StringBuilder, the code reduces the need for dynamic resizing during string concatenation, which can improve efficiency, especially in frequently called methods or when dealing with large strings.
As noted by Intellij:
Reports attempts to instantiate a new StringBuffer or StringBuilder object without specifying its initial capacity.
If no initial capacity is specified, a default capacity is used, which will rarely be optimal. Failing to specify the initial capacity for StringBuffer may result in performance issues if space needs to be reallocated and memory copied when the initial capacity is exceeded.
Linked Issue
Changes
Manual Testing Steps
Screenshots (Optional)
Additional Context (Optional)
AI Disclosure
Checklist
just ui checkandjust api check.Summary by CodeRabbit