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

Virtus::Lite #299

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Virtus::Lite #299

wants to merge 2 commits into from

Conversation

tb0yd
Copy link

@tb0yd tb0yd commented Nov 11, 2014

Hi, first I want to say thanks for all your hard work on this crucial gem.

This is a smaller, more light-weight implementation of the core Virtus DSL. The idea is to reduce the number of supported features in order to improve performance. We would like the current maintainer(s) of Virtus to review our work and consider incorporating it. Thanks again!

@solnic
Copy link
Owner

solnic commented Nov 12, 2014

Hey, thanks, I know Virtus can be too slow and it'll be addressed in 2.0. I'm not entirely sure if merging this in is a good idea though. I'd probably suggest releasing it as a separate project for people who experience performance troubles with Virtus. In 2.0 Virtus will use a rewritten version of coercible which will be much faster as well as configurable way of handling keys in attribute hashes (string vs symbol, right now it tries to support both at the same time which slows things down too).

@davidcole
Copy link

Awesome. That's good news. What is the timeframe for the 2.0 release? We're anxiously waiting for it, but there doesn't seem to be any recent activity on the repo or a 2.0 branch. That has us wanting to help out, if we can.

Might this module be something to tide us over until 2.0 is available? I know other developers are in the same situation.

@solnic
Copy link
Owner

solnic commented Nov 12, 2014

@davidcole my priority is ROM now so I don't have time for Virtus, unfortunately. I'd like to write down roadmap for 2.0 which might be a good start for people who're willing to help. Frankly it may turn out that it won't be that much work at the end of the day since what I'm planning to do is essentially remove a bunch of features, simplify type-constant lookup by using an explicit type-registration system and simply match symbol names to constants during finalization and rewrite coercible to make it faster (it's very slow because it uses a lot of regexp matching which isn't really needed in most of the cases, at least that's my theory for now ;)). OK this may sound like a lot of work but it really isn't :)

@davidcole
Copy link

Haha. Very cool. I like the direction you are heading with those ideas, and I understand the difficulty of working on multiple projects with limited time. If you could get us a rough road map and spec out your ideas, at least in part, we'll see what we can do to help make it happen.

We use Virtus extensively, including in new development, and would be very interested in helping out.

What features are you wanting to remove in 2.0? I like removing features. :)

@solnic
Copy link
Owner

solnic commented Nov 12, 2014

The only thing that I know for sure right now is removing crazy constant lookup logic. I'd have to think about other things a little more before I can come up with a list of things to be removed (or moved out).

I'll come up with a roadmap for 2.0 with more details after releasing ROM later this month, would that work for you?

@davidcole
Copy link

Right on. Yes, that timing works well. I look forward to it!

@solnic
Copy link
Owner

solnic commented Nov 12, 2014

Thanks, that's great to hear. It's really motivating for me to know there are people not only using this library extensively but also who are willing to help. We'll be in touch 😄

@tjstankus
Copy link

Thanks for the feedback @solnic and thanks for getting this conversation going @davidcole. I'd also be very interested in a Virtus 2.0 roadmap. @davidcole and I work at the same company and use Virtus extensively in various projects. We released a simple mapper extension recently. We're heavily invested and our usage of Virtus is growing. All of which is to say, we'd be happy to help however we can with 2.0.

@solnic
Copy link
Owner

solnic commented Nov 21, 2014

@tjstankus that's really great to know! Since you built a mapper extension for Virtus then it might be worth for you to check out ROM. Virtus 2.0 will be built with ROM's integration in mind, it'll be definitely one of my main priorities to make them play nice together. There's a functionality overlap in those projects so it'll be interesting to see how ROM can leverage Virtus' features to avoid duplication.

@tjstankus
Copy link

@solnic Yes, I've been keeping an eye on ROM and look forward to trying it out. Our data source is, more or less, a big, flat JSON hash, which we assemble into model objects. Our mapper provides hash key translation and lambdas when a simple key translation isn't enough. Once we get to 1.0-ish I plan to take another look at ROM. I think our problems aren't too business-specific and we could ultimately benefit from using a more fully-baked datamapper-ish approach.

@solnic
Copy link
Owner

solnic commented Nov 26, 2014

@tjstankus could you tell me when a simple key translation isn't enough?

@mbj
Copy link
Collaborator

mbj commented Nov 27, 2014

@solnic There are cases you want to lift or push keys down or up in the hierarchy. This is especially common when mapping non trivial APIs data formats to/from rich domain models. A simple key translation is not enough here (hint, morpher :P).

@tjstankus
Copy link

@solnic I'm not sure if "simple" is the best word, but for virtus-mapper we use lambdas, for example, when we need to dig through nested keys, like in the address attribute shown below:

class Person
  include Virtus.model
  include Virtus::Mapper

  attribute :first_name, String
  attribute :last_name, String, from: :surname
  attribute :address, String, from: lambda { |atts| atts[:address][:street] rescue '' }
end

person = Person.new({ first_name: 'John',
                      surname: 'Doe',
                      address: { 'street' => '1122 Boogie Avenue' } })
person.first_name # => 'John'
person.last_name # => 'Doe'
person.address # => '1122 Boogie Avenue'
person.surname # => NoMethodError

@davidcole
Copy link

Hey, @solnic. Happy Holidays!

We'd like to get started seeing what we can do to increase the performance of Virtus proper.

You mentioned these items:

  • removing features
  • simplifying type-constant lookup
  • rewriting coercible for performance

Is there some small bit we can get started on? What features can we start removing?

@tb0yd
Copy link
Author

tb0yd commented Dec 17, 2014

+1 -- I could also devote some time to help develop Virtus 2.0.

@solnic -- Is there anything we can do to improve the performance of v1, that would also be helpful for v2.0 going forward? Would you be open to accepting PRs for v1, just for some small performance boosts here and there?

You mentioned that we could try releasing the Virtus::Lite code as a separate project. We actually did discuss that within our team, but turns out it's not an option for us. We are heavily invested in Virtus and don't want to cut away from it.

Thanks again!

@solnic
Copy link
Owner

solnic commented Dec 17, 2014

@davidcole @tb0yd hey sorry for not responding, just wanted to let you know that this is on my radar and I will reply before end of the week. There's just a lot to say and I'm too busy with other stuff now :) Please give me a couple more days to properly respond. And thanks for offering help, I'm really happy about this!

@davidcole
Copy link

👍

@solnic
Copy link
Owner

solnic commented Dec 24, 2014

@tb0yd @davidcole hey could you join rom-rb/chat on gitter so that we could talk about virtus? I'd like to know what you need so that we could prioritize tasks. I believe the best thing to do is to start with coercible and removing regexp parsing (making it optional through configuration). This should give it a perf kick. Maybe we could also change coercible design, it's kinda heavy right now. I would like to use ruby built-in coercion wherever possible and use custom logic only when it's needed (date parsing, boolean casting etc.).

I would also probably suggest running some benchmarks with profiling. I only have a vague memory of doing so and remember coercible was the biggest culprit. It'd be worth to see it again though.

As far as virtus 2.0 goes I updated the milestone description: https://github.com/solnic/virtus/milestones

If you are up to do any of those tasks please let me know. It is definitely a live-chat material though :)

"Unfortunately" I'm very busy with ROM these days pushing really hard to release 1.0.0 soon. I will have time for Virtus once ROM is stabilized.

Please also remember that there's a big functionality overlap between ROM and Virtus. Something common and reusable will come out of that eventually.

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.

5 participants