Skip to content

Conversation

anfilat
Copy link

@anfilat anfilat commented Jun 13, 2020

Purpose / Goal

Refactoring XML parser for more speed

Type

Please mention the type of PR

  • [ ]Bug Fix
  • [x]Refactoring / Technology upgrade
  • [ ]New Feature

Benchmark before

Running Suite: XML Parser benchmark
validation : 24472.96676651558 requests/second
xml to json : 20323.82787435037 requests/second
xml to json + json string : 18513.34418233639 requests/second
xml to json string : 3257.8999597267316 requests/second
xml2js : 6503.5277873583655 requests/second

Benchmark after

Running Suite: XML Parser benchmark
validation : 25254.443972895737 requests/second
xml to json : 37566.136707569676 requests/second
xml to json + json string : 32032.6373744502 requests/second
xml to json string : 3351.0517807181677 requests/second
xml2js : 7196.912529781278 requests/second

Note : Please ensure that you've read contribution guidelines before raising this PR. If your PR is in progress, please prepend [WIP] in PR title. Your PR will be reviewed when [WIP] will be removed from the PR title.

Bookmark this repository for further updates.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 93.443% when pulling fec04ea on anfilat:master into c665954 on NaturalIntelligence:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 93.443% when pulling fec04ea on anfilat:master into c665954 on NaturalIntelligence:master.

@amitguptagwl
Copy link
Member

Thanks for your changes. But it looks like that the after benchmark is increased for xml2js as well. So I'm not sure if there is any performance improvement. However, I'll check it on my local for both of the codes as soon as I get time.

@anfilat
Copy link
Author

anfilat commented Jun 14, 2020

@amitguptagwl Yes, it's my fail. I tested my changes with node 10 and forgot to switch back to to node 14. Real benchmarks are

Before

Running Suite: XML Parser benchmark
validation : 24392.817587933325 requests/second
xml to json : 20444.579203943587 requests/second
xml to json + json string : 18832.37180819154 requests/second
xml to json string : 3260.372110704194 requests/second
xml2js : 6531.4937847609235 requests/second

After

Running Suite: XML Parser benchmark
validation : 24668.64955496943 requests/second
xml to json : 40247.82636682904 requests/second
xml to json + json string : 34608.35386438647 requests/second
xml to json string : 3539.056066557331 requests/second
xml2js : 6614.874820643066 requests/second

@amitguptagwl
Copy link
Member

No doubt, I have left the room for some improvements from v3.7.0. But it is more than my expectation.

Wow. Let me validate. Thanks for your effort.

@amitguptagwl
Copy link
Member

I've glanced at your changes. I'm actually working on the next version on FXP which uses classes. But its speed is only 25K(parsing + validation). However, the changes you did is giving the performance up to 40k (without validation). I tested with node 10 and 11. And you have tested with node 14.

So let me check and compare thoroughly. Thanks for your effort.

@amitguptagwl
Copy link
Member

@anfilat I have pushed the v4 branch which already has performance improvement. We can check further if other improvements can be done.

@anfilat
Copy link
Author

anfilat commented Aug 9, 2020

Ok

@anfilat anfilat closed this Aug 9, 2020
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.

3 participants