Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix stack overflow when decoding deeply nested structures like arrays and maps #51

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fix stack overflow when decoding deeply nested structures like arrays and maps #51

wants to merge 5 commits into from

Conversation

MarshalX
Copy link
Owner

@MarshalX MarshalX commented Nov 18, 2024 •
edited
Loading

fixes #9

It uses 2 approaches. First for Python 3.8 and second for Python 3.9+. The first one hard limits recursion to 1k (same as python by default). The second one respects sys.setrecursionlimit. decode_dag_cbor function could now raise RecursionError

CI fails because of temporary disabled endpoint by bsky team to deal with network load

@MarshalX MarshalX mentioned this pull request Nov 18, 2024
Open
@codspeed-hq CodSpeed HQ
Copy link

codspeed-hq bot commented Nov 23, 2024

CodSpeed Performance Report

Merging #51 will degrade performances by 34.36%

Comparing fix-overflow-on-deep-structures (4d2449e) with main (7bf9fb9)

Summary

❌ 65 regressions
✅ 127 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main fix-overflow-on-deep-structures Change
❌ test_dag_cbor_decode[roundtrip02.json] 16.7 µs 18.7 µs -10.46%
❌ test_dag_cbor_decode[roundtrip03.json] 16.7 µs 18.7 µs -11.02%
❌ test_dag_cbor_decode[roundtrip04.json] 17.4 µs 19.5 µs -10.33%
❌ test_dag_cbor_decode[roundtrip05.json] 18.2 µs 20.3 µs -10.54%
❌ test_dag_cbor_decode[roundtrip06.json] 15.2 µs 17.4 µs -12.92%
❌ test_dag_cbor_decode[roundtrip08.json] 16.8 µs 18.7 µs -10.03%
❌ test_dag_cbor_decode[roundtrip10.json] 20 µs 22.7 µs -11.81%
❌ test_dag_cbor_decode[roundtrip11.json] 16.6 µs 18.5 µs -10.27%
❌ test_dag_cbor_decode[roundtrip13.json] 17.4 µs 19.5 µs -10.62%
❌ test_dag_cbor_decode[roundtrip17.json] 17.3 µs 19.3 µs -10.26%
❌ test_dag_cbor_decode[roundtrip18.json] 17.4 µs 19.3 µs -10.07%
❌ test_dag_cbor_decode[roundtrip19.json] 17.4 µs 19.3 µs -10.32%
❌ test_dag_cbor_decode[roundtrip20.json] 16.9 µs 18.9 µs -10.45%
❌ test_dag_cbor_decode[roundtrip21.json] 16.8 µs 19.1 µs -12.03%
❌ test_dag_cbor_decode[roundtrip22.json] 16.8 µs 18.9 µs -11.28%
❌ test_dag_cbor_decode[roundtrip23.json] 17.4 µs 19.7 µs -11.71%
❌ test_dag_cbor_decode[roundtrip24.json] 18.3 µs 20.6 µs -11.14%
❌ test_dag_cbor_decode[roundtrip25.json] 16.8 µs 18.8 µs -10.67%
❌ test_dag_cbor_decode[roundtrip26.json] 16.8 µs 19 µs -11.35%
❌ test_dag_cbor_decode[roundtrip27.json] 16.7 µs 18.8 µs -11.08%
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@MarshalX
Copy link
Owner Author

R.I.P

image

@MarshalX MarshalX mentioned this pull request Dec 2, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Segfault on decoding of nested lists and maps
1 participant
@MarshalX