-
Notifications
You must be signed in to change notification settings - Fork 1
⚡ perf(ingestion): bulk parse JSONL files in _parse_jsonl #133
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -190,23 +190,42 @@ def _parse_json(path: Path) -> str: | |
|
|
||
| def _parse_jsonl(path: Path) -> str: | ||
| lines: List[str] = [] | ||
| with path.open("r", encoding="utf-8", errors="replace") as handle: | ||
| for raw in handle: | ||
| raw_line = raw.strip() | ||
| if not raw_line: | ||
| continue | ||
| try: | ||
| payload = json.loads(raw_line) | ||
| except json.JSONDecodeError: | ||
| lines.append(raw_line) | ||
| continue | ||
|
|
||
| raw_text = path.read_text(encoding="utf-8", errors="replace") | ||
| valid_lines = [line.strip() for line in raw_text.splitlines() if line.strip()] | ||
| if not valid_lines: | ||
| return "" | ||
|
|
||
| # Fast path: bulk parse | ||
| try: | ||
| bulk_str = "[" + ",".join(valid_lines) + "]" | ||
| payloads = json.loads(bulk_str) | ||
| for payload in payloads: | ||
| extracted: List[str] = [] | ||
| _extract_chat_lines(payload, extracted) | ||
| if extracted: | ||
| lines.extend(extracted) | ||
| else: | ||
| lines.append(json.dumps(payload, ensure_ascii=False, sort_keys=True)) | ||
| return _truncate_output("\n".join(lines)) | ||
| except json.JSONDecodeError: | ||
| pass | ||
|
|
||
| # Fallback to line-by-line parsing | ||
| lines.clear() | ||
| for raw_line in valid_lines: | ||
| try: | ||
| payload = json.loads(raw_line) | ||
| except json.JSONDecodeError: | ||
| lines.append(raw_line) | ||
| continue | ||
|
|
||
| extracted: List[str] = [] | ||
| _extract_chat_lines(payload, extracted) | ||
| if extracted: | ||
| lines.extend(extracted) | ||
| else: | ||
| lines.append(json.dumps(payload, ensure_ascii=False, sort_keys=True)) | ||
|
|
||
| return _truncate_output("\n".join(lines)) | ||
|
Comment on lines
+194
to
230
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current implementation introduces a significant memory risk and code duplication. Reading the entire file into memory with Additionally, the extraction logic is duplicated between the bulk parse and fallback paths. Consider refactoring to use a shared processing function and a memory-efficient fallback that iterates over the file handle directly, while keeping the bulk optimization for smaller files. def process_payload(p):
extracted: List[str] = []
_extract_chat_lines(p, extracted)
if extracted:
lines.extend(extracted)
else:
lines.append(json.dumps(p, ensure_ascii=False, sort_keys=True))
# Fast path: bulk parse for small/medium files
# We use a size check to prevent OOM on massive files
if path.stat().st_size < 50 * 1024 * 1024:
try:
raw_text = path.read_text(encoding="utf-8", errors="replace")
valid_lines = [l.strip() for l in raw_text.splitlines() if l.strip()]
if valid_lines:
payloads = json.loads("[" + ",".join(valid_lines) + "]")
for payload in payloads:
process_payload(payload)
return _truncate_output("\n".join(lines))
except (json.JSONDecodeError, MemoryError):
lines.clear()
# Fallback or Large File Path: Memory-efficient line-by-line parsing
with path.open("r", encoding="utf-8", errors="replace") as handle:
for raw in handle:
raw_line = raw.strip()
if not raw_line:
continue
try:
process_payload(json.loads(raw_line))
except json.JSONDecodeError:
lines.append(raw_line)
return _truncate_output("\n".join(lines))References
|
||
|
|
||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
Building
bulk_strby joining every JSONL record into one giant array forces at least one extra full-copy allocation of the file contents, andjson.loadsthen materializes the whole array at once. For large JSONL inputs this can raiseMemoryError(or trigger OOM) before the fallback runs, because onlyJSONDecodeErroris caught, so ingestion now fails where the previous streaming loop could still process the file incrementally.Useful? React with 👍 / 👎.