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

return hashes instead of RecursiveObjectStruct #197

Closed
grosser opened this issue Oct 4, 2016 · 6 comments
Closed

return hashes instead of RecursiveObjectStruct #197

grosser opened this issue Oct 4, 2016 · 6 comments

Comments

@grosser
Copy link
Contributor

grosser commented Oct 4, 2016

less magic ... less work/coversion ... more performance ?
... sounds good ? ... maybe optional via config ?

@simon3z
Copy link
Collaborator

simon3z commented Oct 4, 2016

cc @moolitayer

@moolitayer
Copy link
Collaborator

moolitayer commented Oct 5, 2016

This would be at most an optional (global instance config value that we would need to add) optimization (over to_h). I'm not sure if it is worth the trouble. if we go down this path we should consider input/output together.

Update: #197 (comment)

@jrafanie
Copy link
Member

maybe optional via config ?

I'd imagine that would be hard because you'd have to support both configurations, dot notation and hash access... Either way, if the performance is that awful, maybe we have no choice but use the simplest solution using hashes... although @moolitayer brings up a great point to consider how we're requesting data and parsing it. If we can request it json or another format that's faster or uses significantly less memory or both, we should consider that path.

@moolitayer
Copy link
Collaborator

moolitayer commented Aug 2, 2017

Just to be clear, given my previous comment, due to actually meeting performance problems I believe getting rid of ROS is totally worth having to deal with backward compatibility problems. (an example is changes in input & output of an update_x method).

personally I would ack a PR doing pure hashes per flag (and maybe in a future major version we will be able to change default)

Any way for our manageiq usage, what I understand from people doing performance tuning (@cben @Ladas and others) is that right now we have more significant bottlenecks

@moolitayer
Copy link
Collaborator

See #262, add option for raw and avoids ROS (and JSON parsing altogether)

@cben
Copy link
Collaborator

cben commented Mar 4, 2018

I think your #306 as: :parsed & as: :parsed_symbolized plus #299 allowing to set as: for whole client covered this well 👏. Can this be closed?

@grosser grosser closed this as completed Mar 4, 2018
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

No branches or pull requests

5 participants