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

change the JSON parser to the native one #142

Open
tremblap opened this issue Oct 5, 2022 · 8 comments
Open

change the JSON parser to the native one #142

tremblap opened this issue Oct 5, 2022 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@tremblap
Copy link
Member

tremblap commented Oct 5, 2022

as disucssed with @weefuzzy we had a pre 3.6 home-made parser. we could implement it as a fall back with try like in FluidSound, or just remove it and replace by the following code which is much faster for large DS to dump to dict for instance (via the tempfile)

(
~parse = {
    var str = File.readAllString("zepath");
    var d = str.parseJSON; 
    d
};
~dic = ~parse.value; 
~dic.postln; 
)
@tremblap tremblap added the enhancement New feature or request label Oct 5, 2022
@weefuzzy
Copy link
Member

weefuzzy commented Oct 5, 2022

Looks like it was changed to the handrolled thing in c1f7725 because the native parsing was enforcing a change of types somewhere undesirable.

@weefuzzy
Copy link
Member

weefuzzy commented Oct 5, 2022

@g-roma do you happen to remember what parseYAMLFie was doing that was undesirable?

@g-roma
Copy link
Member

g-roma commented Oct 5, 2022

I can't remember, but there was some problem.

@weefuzzy weefuzzy self-assigned this Oct 5, 2022
@weefuzzy
Copy link
Member

weefuzzy commented Oct 5, 2022

The problem seems to be that parseYAML and friends make everything a String, e.g.

d = "{\"foo\":[1.0,2.0,3,4,5]}".parseJSON
d["foo"].do{|x| x.class.postln}
-> Dictionary[ (foo -> [ 1.0, 2.0, 3, 4, 5 ]) ]
String
String
String
String
String

🤦

So, I guess the alternative to what we're doing now is to walk the whole Stringified Dictionary after faster parsing and coerce values back to float where appropriate. That may or may not be faster than the current implementation.

@weefuzzy
Copy link
Member

weefuzzy commented Oct 5, 2022

see supercollider/supercollider#1154 which has been open since 2014

looks like yaml-cpp doesn't provide a nice way to determine whether a scalar is a string, int or float (see https://stackoverflow.com/questions/19994312/obtain-type-of-value-stored-in-yamlnode-for-yaml-cpp)

@tremblap
Copy link
Member Author

Indeed @weefuzzy at the moment I can read faster but I cannot use any of it without converting to float the strings... not super useful :) Slow it stays then, until we find a way around this.

@tremblap
Copy link
Member Author

actually, running

i["cols"] = i["cols"].asInteger;

)

(
x = Main.elapsedTime;
i["data"].keysValuesDo{|a,b|i["data"][a] = b.asFloat};
(Main.elapsedTime - x).postln;
)

takes 100% cpu for quite some time. So I don't know what the answer to

That may or may not be faster than the current implementation.

is yet...

@tremblap
Copy link
Member Author

ok our version is faster than parsing the dict to itself - although it suddenly occured to me that maybe I'm making an infinite loop by parsing over itself - another test is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants