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

improve json range support #1906

Merged
merged 2 commits into from Sep 15, 2017
Merged

Conversation

John-Colvin
Copy link
Contributor

A very simple translation of skipNumber to the range API.

@John-Colvin John-Colvin force-pushed the skip_number_range branch 2 times, most recently from cde8603 to 42769d2 Compare August 30, 2017 10:20
@UplinkCoder
Copy link
Contributor

@John-Colvin please profile the performance consequences of this change.
It seems like it'd hurt quite a bit. please profile with -Os as well

@s-ludwig
Copy link
Member

One small performance improvement would be to skip incrementing idx for ranges that define length and use takeExactly(sOrig.length - s.length) instead. But benchmarking is a topic that I need to get back to anyway. I did a quick test at DConf this year and performance was really bad, whereas I remember pretty good numbers from the last tests.

@John-Colvin
Copy link
Contributor Author

John-Colvin commented Aug 31, 2017

The major performance problem is now fixed by operating on ubyte[] instead of auto-decoded strings (Oh how I hate them).

In debug mode assumeUTF verifies utf, which is of course expensive. In release, that check doesn't happen.

I would have preferred to use byCodeUnit instead, but dmd isn't so great at optimising that.

@s-ludwig
Copy link
Member

Thanks, John! The performance implication of the additional index is supposedly small enough.

The more important question IMO is anyway to see what the future of std_data_json is. There were some points brought up that made it difficult to continue the Phobos integration route, so it might make sense to establish it as an independent library and let it slowly replace this module.

@s-ludwig s-ludwig merged commit a67bf9f into vibe-d:master Sep 15, 2017
@John-Colvin John-Colvin deleted the skip_number_range branch September 19, 2017 10:42
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.

None yet

3 participants