8000 Switch indexeddb backend to sqlite and improve IPC messaging by arihant2math · Pull Request #38187 · servo/servo · GitHub
[go: up one dir, main page]
Skip to content

Switch indexeddb backend to sqlite and improve IPC messaging #38187

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
Merged
merged 3 commits into from
Aug 16, 2025
Merged

Switch indexeddb backend to sqlite and improve IPC messaging #38187

merged 3 commits into from
Aug 16, 2025

Conversation

arihant2math
Copy link
Contributor
@arihant2math arihant2math commented Jul 21, 2025 •
edited
Loading
  • Use sqlite instead of heed. (one indexed database = one sqlite database)
  • Implement the backend for indexes
  • Use keyranges where needed (as specified by the spec)
  • Implement getKey
  • Fix channel error messaging (led to a bunch of changes to how async requests are handled)

Note: components/net/indexeddb/engines/sqlite/serialize.rs is unused; I can delete it if needed.

Testing: Switching to sqlite eliminated many panics (exposing some new failures).
Fixes: #38040

@servo-wpt-sync
Copy link
Collaborator

🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync.

1 similar comment
@servo-wpt-sync
Copy link
Collaborator

🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync.

@arihant2math arihant2math force-pushed the idb-index branch 2 times, most recently from 82e6ed1 to 43f3335 Compare July 22, 2025 06:46
@servo-wpt-sync
Copy link
Collaborator

🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#53885) with upstreamable changes.

@servo-wpt-sync servo-wpt-sync mentioned this pull request Jul 22, 2025
Closed
@arihant2math
Copy link
Contributor Author

servo-wpt-sync should really not open PRs in the wpt repo for a draft pr.

@arihant2math
Copy link
Contributor Author

Note to self: use git-cherrypick next time.

@servo-wpt-sync
Copy link
Collaborator

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#53885).

@servo-wpt-sync
Copy link
Collaborator

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#53885).

@servo-wpt-sync
Copy link
Collaborator

🤖 This change no longer contains upstreamable changes to WPT; closed existing upstream pull request (web-platform-tests/wpt#53885).

@arihant2math arihant2math marked this pull request as ready for review July 24, 2025 10:24
@arihant2math arihant2math requested a review from gterzian as a code owner July 24, 2025 10:24
@arihant2math
Copy link
Contributor Author
arihant2math commented Jul 24, 2025 •
edited
Loading

@jdm I think the pr is ready for review when you get the time.

This was referenced Jul 24, 2025
Closed
Closed
Closed
@arihant2math arihant2math mentioned this pull request Jul 24, 2025
Closed
@jdm
Copy link
Member
jdm commented Jul 24, 2025

@jdm I think the pr is ready for review when you get the time.

I really appreciate the work you're doing here, but batching multiple PRs together into one PR with 26 commits is not going to work for me. Incremental work that can exist in its own PR is easier to 1) review, 2) understand, and 3) identify regressions. I would really appreciate if you could extract this work into separate PR 8000 s.

@arihant2math
Copy link
Contributor Author

I can probably branch off part of key ranges and the cmp method, but the bulk of the work is tightly coupled (I rewrote the messaging system when I rewrote the backend system to reduce the amount of work needed).

@arihant2math arihant2math changed the title More indexeddb implementation Switch indexeddb backend to sqlite and improve messaging system Jul 25, 2025
@arihant2math arihant2math changed the title Switch indexeddb backend to sqlite and improve messaging system Switch indexeddb backend to sqlite and improve ipc messaging Jul 25, 2025
@arihant2math arihant2math changed the title Switch indexeddb backend to sqlite and improve ipc messaging Switch indexeddb backend to sqlite and improve IPC messaging Jul 25, 2025
@arihant2math arihant2math force-pushed the idb-index branch 3 times, most recently from 883dcf6 to bf9d060 Compare July 26, 2025 04:45
@arihant2math arihant2math force-pushed the idb-index branch 2 times, most recently from b761237 to fe4394d Compare August 10, 2025 09:46
@arihant2math
Copy link
Contributor Author

Fixed merge conflicts, should be good to go again.

@arihant2math arihant2math mentioned this pull request Aug 10, 2025
Closed
jdm
jdm reviewed Aug 10, 2025
View reviewed changes
Copy link
Member
@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed everything but engines/sqlite.rs before I ran out of time. I'll try to get back to this by tomorrow.

jdm
jdm requested changes Aug 12, 2025
View reviewed changes
Copy link
Member
@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking pretty close to mergeable!

7440
@arihant2math arihant2math force-pushed the idb-index branch 5 times, most recently from f63ee3a to 9b979d9 Compare August 16, 2025 02:18
@arihant2math arihant2math requested a review from jdm August 16, 2025 02:27
@jdm
Copy link
Member
jdm commented Aug 16, 2025

This can merge once ./mach test-tidy passes.

@arihant2math
Port to sqlite
6b57d81 
Signed-off-by: Ashwin Naren <arihant2math@gmail.com>
@arihant2math
address review
45a30c8 
Signed-off-by: Ashwin Naren <arihant2math@gmail.com>
@arihant2math
address review
2efb683 
Signed-off-by: Ashwin Naren <arihant2math@gmail.com>
@arihant2math
Copy link
Contributor Author

Alright, should be fixed.

@jdm jdm added this pull request to the merge queue Aug 16, 2025
Merged via the queue into servo:main with commit fc3fece Aug 16, 2025
22 checks passed
@arihant2math arihant2math deleted the idb-index branch August 16, 2025 14:42
@arihant2math arihant2math added the A-content/indexeddb IndexedDB implementation issues. label Aug 17, 2025
PotatoCP pushed a commit to PotatoCP/servo that referenced this pull request Aug 20, 2025
@arihant2math @PotatoCP
Switch indexeddb backend to sqlite and improve IPC messaging (servo#3…
651d292 
…8187)

- Use sqlite instead of heed. (one indexed database = one sqlite
database)
- Implement the backend for indexes
- Use keyranges where needed (as specified by the spec)
- Implement `getKey`
- Fix channel error messaging (led to a bunch of changes to how async
requests are handled)

Note: `components/net/indexeddb/engines/sqlite/serialize.rs` is unused;
I can delete it if needed.

Testing: Switching to sqlite eliminated many panics (exposing some new
failures).
Fixes: servo#38040

---------

Signed-off-by: Ashwin Naren <arihant2math@gmail.com>
@jschwe jschwe mentioned this pull request Aug 27, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrobinson mrobinson mrobinson left review comments

@jdm jdm jdm approved these changes

+1 more reviewer

@janvarga janvarga janvarga left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
A-content/indexeddb IndexedDB implementation issues.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Switch to sqlite for indexeddb backend
6 participants
@arihant2math @servo-wpt-sync @jdm @mrobinson @janvarga @webbeef
0