Skip to content

rpc: handle XML-RPC with JSON-RPC logics#65

Open
jesec wants to merge 1 commit into
masterfrom
refactor/xml-as-json
Open

rpc: handle XML-RPC with JSON-RPC logics#65
jesec wants to merge 1 commit into
masterfrom
refactor/xml-as-json

Conversation

@jesec

@jesec jesec commented Jul 3, 2023

Copy link
Copy Markdown
Owner

This allows us to centralize on a single set of RPC logics, sharing the exception handling, error codes/messages and behaviors.

This improves consistency, reduces maintenance burden, makes future extension to RPC parts easier. In addition, we can ditch the dependency on decade-old xmlrpc-c library.

As for the performance, we performed a test with 10,000 torrents in a typical d.multicall2 call, which showed JSON-RPC is 3x faster, compared to XML-RPC (xmlrpc-c). Meanwhile, rapidxml is 3-9x faster, compared to expat/libxml. That means even with the conversion, we shouldn't see much of a performance ding. We may even have a win.

This allows us to centralize on a single set of RPC logics, sharing
the exception handling, error codes/messages and behaviors.

This improves consistency, reduces maintenance burden, makes future
extension to RPC parts easier. In addition, we can ditch the dependency
on decade-old xmlrpc-c library.

As for the performance, we performed a test with 10,000 torrents in
a typical d.multicall2 call, which showed JSON-RPC is 3x faster,
compared to XML-RPC (xmlrpc-c). Meanwhile, rapidxml is 3-9x faster,
compared to expat/libxml. That means even with the conversion, we
shouldn't see much of a performance ding. We may even have a win.
@codecov

codecov Bot commented Jul 3, 2023

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 1.58228% with 311 lines in your changes missing coverage. Please review.
✅ Project coverage is 7.11%. Comparing base (88fd968) to head (5ae8972).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
include/utils/rapidxml/rapidxml_print.hpp 0.00% 192 Missing ⚠️
src/rpc/rpc_xml.cc 0.00% 110 Missing ⚠️
include/rpc/rpc_xml.h 33.33% 4 Missing ⚠️
src/rpc/rpc_json.cc 0.00% 4 Missing ⚠️
src/rpc/scgi.cc 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           master     #65      +/-   ##
=========================================
- Coverage    7.95%   7.11%   -0.85%     
=========================================
  Files         162     171       +9     
  Lines       10497   12006    +1509     
=========================================
+ Hits          835     854      +19     
- Misses       9662   11152    +1490     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kannibalox

Copy link
Copy Markdown
Contributor

Is there room for a shim for system.multicall? I can take a stab at it myself if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants