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.

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

XML parsing dependency migration #312

Open
amitguptagwl opened this issue Jan 15, 2019 · 5 comments
Open

XML parsing dependency migration #312

amitguptagwl opened this issue Jan 15, 2019 · 5 comments

Comments

@amitguptagwl
Copy link

I'm raising this issue as suggested by @XiaoningLiu

Is your feature request related to a problem? Please describe.
Not exactly. But for improvements.

Describe the solution you'd like
We're currently dependent on xml2js library to parse XML. Fast-XML-Parser is a better alternative to this.

Additional context
Here is the comparison of both packages to parse XML data.

file size fxp 3.0 parser (rps) xml2js 0.4.19 (rps)
1.5k 14032.09323 4615.930805
1.5m 13.23366098 5.90682005
13m 1.135582008 -1
1.3k with CDATA 43160.52342 8398.556349
1.3m with CDATA 52.68877009 7.966000795
1.6k with cdata,prolog,doctype 41433.98547 7872.399268
98m 0.2600104004 -1

In the below chart, X-axis is about XML data size, and Y-axis is for throughput or number of parses per second.

fxp vs xml2js

@ghost
Copy link

ghost commented Jan 15, 2019

@amitguptagwl Is your table and chart data incorrect? You said that fxp is better than xml2js, but your chart and data show the opposite.

@amitguptagwl
Copy link
Author

@daschult how is it opposite? Eg FXP can parse 15k XML data 14032 times in a second but xml2js can parse the same data in 4616 times only.

@ghost
Copy link

ghost commented Jan 16, 2019

Ah, I wasn't familiar with rps in this context. I assumed it was some kind of measure of time, which would mean that fxp was much slower than xml2js, but if that's how many parses per second (Rotations Per Second, I'm guessing?), then that makes more sense.
Yes, it seems like we'll have to think about whether this makes sense for us to do.

@ghost ghost added the customer-reported label Jan 16, 2019
@ramya-rao-a
Copy link
Contributor

@xirzec, @jeremymeng, @joheredi Thoughts here?

@jeremymeng
Copy link
Member

I had given fxp a try but it seems non-trivial to replace xml2js as fxp doesn't provide similar settings to those of xml2js that we used. However if we could make fxp work and if it doesn't have the circular reference issue, then it would be great to the React-Native story.

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

No branches or pull requests

3 participants