Improve performance of xhr

[vertis]

As mentioned in #5329 and 3756a3f

This change is needed to improve the performance of xhr? in rubinius and jruby.

[stevenh512]

👍 let's forget the semantics and just face the fact that, regardless of whether or not both might technically be correct.. it's already been demonstrated that in this particular instance returning actual true or false is a performance benefit on at least two popular implementations of Ruby. That's not always true (or even true or 0 😁), but in this case it is. Since true and false are just as "truthy" and "falsy" as 0 and nil and either version is easy to read and understand, why not go with the version that's faster?

[josevalim]

The current performance of xhr? is fine, this patch will not bring a performance improvement to any application. Thanks for the pull request anyway.

Note: We don't do such micro performance optimizations in Rails, for instance, we write map(&:to_s), we don't bother having to write map { |x| x.to_s } even though the former is "slower", because it simply doesn't affect any app at the end of the day. Please send pull requests optimizing ARel, the rendering stack, etc. and I will gladly discuss and merge it.

[vertis]

@josevalim please review the previous pull request and reverted commit. @fxn clearly stated that if this was changed specifically for the purpose of improving performance, then it would be acceptable.

[fxn]

@vertis but we are the core team and we discuss things. I expressed an opinion there, but I concur with José once I 've seen his point of view.

[vertis]

@fxn I would have been extremely surprised had you merged it.

[stevenh512]

@fxn did you discuss it with @tenderlove when you reverted a pull request that he seemed to have no problem with (in fact judging by his comments on the actual pull request I'd say he loved it)?

@vertis yeah, no surprise here, as tempted as I was to make this same pull request just to prove a point (and the fact that this PR wasn't merged proves that point anyway), I knew it would be a complete waste of time that I could spend contributing somewhere else.

[brainopia]

Ruby docs: Regexp#=== – Case Equality—Synonym for Regexp#=~ used in case statements.

Judging by the code of Rubinius and MRI Regexp#=== is almost the same as Regexp#=~. It performs the same logic for search but === accounts for type conversion and returns boolean instead of position – that's all.

There is small chance to benchmark it properly, but @headius perfomed the same benchmark as @tarcieri and had different results: https://gist.github.com/2176643.

So there is no evidence it will improve even roflscale benchmarks. Therefore, I do not think it's appropriate to attempt to play benchmark card in this case.

P.S.

There are a lot of aggressive and emotional arguments addressed at @fxn. It's not nice. It seems some people forgot not only ruby foundations but also the ruby way – "Matz is nice so we are nice".

To these I address: Please respect the people who make the final call and who spent countless number of their own free time to create something you probably use to earn money.

[vertis]

@brainopia it was to prove a point.

I don't feel like @fxn handled the matter appropriately. The attitude portrayed throughout the original pull request was one of a dictator not a collaborator. Lets recap:

• A user came with a problem, resulting in a change from 2.3 to 3.x.
• The user was dismissed and insulted as having his code wrong, despite the fact that it is an accepted practice to return singletons from predicate (?) methods. One doesn't have to look very far in Ruby core to find that the majority of the methods work this way.
• When there was broad support for reverting this method to the previous way of working, @fxn objected for the sake of clarity(?), and later for not wanting to create the impression that they can be relied on to return singletons.
• When a subsequent patch was applied by another core team member, he reverted a perfectly good commit for the sake of winning the argument. In doing so he stated that if it had been done with performance as the message he would have been fine with it.

So here we are.

[fxn]

@vertis I couldn't be more respectful and polite in that pull request.

I didn't insult anyone. The code had a bug, that is a fact you should not take that as an offense to the OP.

I clearly stated that I totally respected that people participating in the thread could have a different point of view about what they prefer to return from predicates. But they do not mandate how Ruby works. They would like Ruby to be like Java. No problem, but it isn't.

The fact that I respect other opinions does not mean I am going to apply the patch or to change the project policy about predicates. Why, because I do not agree with those opinions, and this is my code base. I am not going to your projects to tell you how to write your predicates to match my preferences.

I didn't revert for the sake of winning the argument. I explained the reasons the patch was reverted in the commit message.

@vertis if you calm down and read the thread cold a week later, you'll see you are not being objective in your judgements.

[vertis]

@fxn that's just the problem though, it's not YOUR code base. If anyone, it belongs to DHH. For the rest, anyone that has contributed, or even attempted to contribute holds a certain amount of ownership. Indeed, anyone that uses an application is a stakeholder, and entitled to respect.

I'm comfortable with everything but nil being true, that doesn't mean it's wrong to be more explicit/strict. Indeed, it helps with issues like the conventions being different between languages -- the perfect example being javascript, which is a part of every rails app.

So why then do you have a problem with a more explicit rendering of truth? What benefit do you gain in this instance to having the method return 0? The removal of !! is fine (I can get behind that), however === is a very fitting replacement, it is not only clear, but it also helps someone out who has a 75k line codebase.

@fxn for the record, I am calm. I'm frustrated at your attitude, and the lack of control that the community has over the rails code base. But I'm hardly frothing at the mouth.

[fxn]

@vertis yes, telling it is my code base is a just a way to say it is not the OP's code base, or your code base. I belong to a team and of course Rails is not mine. That goes without saying. And the team already has a policy about how to write predicates. That's why almost all the API documents predicates as generic predicates. There some exceptions documenting singletons, but a little inconsistency is normal in a large code base.

That PR is not about xhr?, is about a project policy. We are not going to special-case this particular predicate. And we are not going to change the project policy.

I don't claim returning singletons is wrong.

The reasons for the revert are explained in the commit message. There is nothing to fix. Do you understand that? Nothing to fix! The current predicate is just fine!

You would like to have it in a different way? I totally respect that.

My attitude is respectful, patient, and polite. I am pretty confident about that.

[stevenh512]

@fxn By putting words in people's mouths, you insult them. Nobody has every said they wanted Ruby to be "like Java" (or like "some imaginary programming language" as you plainly accused me of saying when I never said any such thing).

By saying things like "my code" when referring to a project that has over a thousand contributors, you insult everybody who has ever contributed to Rails or attempted to contribute to Rails. You're from another country and perhaps English isn't your first language, so maybe you get a little slack on that one.. but please choose your words better if you don't want to seem like you're insulting people.

By saying "If this pull request were submitted for performance reasons, I would accept it" when you knew you had no intention of doing so, you not only insult everybody's intelligence, but you prove yourself to be a liar because you gave your word that you would do something when you knew full well that you were going to do the exact opposite. Where I come from a man's word is his bond. You gave your word, you should have done what you said you would do. Now, to me (and I'm sure many others) you've lost all credibility.

By "talking to the core team" about it at a time when the majority of the core team was more than likely in bed asleep or just waking up to start the day (how many did you actually talk to? 3? 4?) and then getting one of those core team members to come in here and close the pull request for you, you insult yourself further, because not only do you look like a liar but now you also look like a coward.

If you take anything I said as an insult, that's your problem, you have insulted me and several other people whether that was your intention or not, and you did lie about your intention to merge this pull request and then found someone else to do your dirty work for you when the pull request was submitted. I only speak the truth, nothing more or less, take it how you will.

While there were parts of this discussion where I'll admit that you have been respectful, for the most part that is not true, your attitude has been rude, condescending, insulting and defensive and you've rejected almost every logicall argument with a "because I can" attitude that is not normal, healthy or acceptable in a community of adults.

[josevalim]

And then getting one of those core team members to come in here and close the pull request for you, you insult yourself further, because not only do you look like a liar but now you also look like a coward.

Are you fucking kidding me? Merging pull requests is the first thing I do in the morning every day (and which I am doing less and less exactly because of the unnecessary stress) and that is to help everyone to get their contributions in, give feedback, and this pull request came my way. And now you say I am doing other's people dirty work? I seriously don't have patience for this shit anymore.

[fxn]

Steven, this has gone too far. I'm done with this thread thanks.

[NZKoz]

Stephen,

Please get a life

[DevL]

Don't feed the trolls.

[stevenh512]

@josevalim my apologies to you.. but since he said he "talked to the core team" at a time when very few of them could possibly be available (and you, being in the same country as him, are one of the ones who could have been available at the time.. anyone in the US or Canada would have most likely been asleep or just waking up to start their day), I assumed he must have talked to you. My bad, I'm sorry for jumping to that conclusion and I probably shouldn't have assumed that.

You personally, I have nothing but respect for, and honestly I was shocked at the thought that you were "doing his dirty work" but that's certainly how his comment, and the timing of this pull request being closed, made it appear. Still, I was wrong to make that assumption without facts just based on the appearance and timing of everything and the general attitude he has displayed throughout this entire discussion.

[NZKoz]

Perhaps your life would be more fulfilling if you didn't let yourself get worked up to the point that the precise semantics of the return value of a method was viewed as some kind of war crime or genocide.

Either way, this whole thing is done, you've all managed to ensure that no matter what the merits of your position are, your changes will be refused because you're all acting like spoiled children whose parents took away their Amex card.

Please take this rage and indignation you're feeling and channel it into bug fixes and performance improvements that matter.

[pi3r]

@stevenh512 please, stop talking for "everybody who has ever contributed to Rails or attempted to contribute to Rails", your are not a superhero defending the "right to be merged"

[stevenh512]

@pi3r I've said that only once that I can recall. Of course, I'm not the only person who has said it (and everyone who has said it was right to say it), so maybe that's where your confusion comes from.

[vertis]

@fxn and other core team members -- I apologise that I have reacted in
anger yesterday, I apologize that my "point making" turned so negative & I
apologise that I have wasted so much of your time. Both in this thread and
others. I believe in feedback and criticism. I do not believe in negativity
and hatefulness.

- Luke

Sent from my phone
On Mar 27, 2012 7:39 PM, "Pierre Nespo" <
reply@reply.github.com>
wrote:

@stevenh512 please, stop talking for "everybody who has ever contributed
to Rails or attempted to contribute to Rails", your are not a superhero
defending the "right to be merged"

---

Reply to this email directly or view it on GitHub:
#5580 (comment)

[fxn]

@vertis no prob, let's have a beer in the next conference we meet at.

[stevenh512]

@NZKoz Actually my life is quite fulfilling (and my name is spelled with a v, not a ph). For what it's worth, I do contribute code and documentations to projects that I feel are worth taking the time to write that code and documentation. At this point my "rage and indignation" as you call it comes only from the fact that I was flat-out and directly lied to.

Again, my apologies to @josevalim for thinking and stating that he was a part of that lie, I see now (and I do trust him and believe him, he's never given me any reason not to) that he was an innocent bystander in all this who just got caught in the crossfire.

My apologies also to @fxn for calling him a coward since it's clear now that he didn't "get someone to do his dirty work" no matter how much it might have appeared that way at the time.. that was out of line and I shouldn't have said it

edit: typo

[alloy]

@fxn Oh shit, so are you now saying that only those that have complained will get a beer?!

Seriously, though, @vertis, I honestly applaud you for that final response :)

[vertis]

Seriously Steven, pull your head in. Nobody deserves to be spoken to like
that. Whatever disagreements @fxn and I have had, he has never acted
inappropriately or rudely. If anything he is the picture of a good steward,
refusing to compromise the code base to serve short term goals.

[vertis]

@alloy we may have disagreements, but the reality is that someone has to be responsible for the code base. We should all be able to discuss differences like adults and realise that there are tens of thousands of users that need to be represented, and the loud voices of a few do not represent a majority.

This pull request was supposed to be a lighthearted attempt to get the patch merged back in, not some spiral of negativity.

[stevenh512]

@vertis I did say something that was clearly wrong and out of line and I did and do apologize for that, to @josevalim and to @fxn.

As far as everything else I've said, I do feel that his attitude was rude and inappropriate throughout this discussion regardless of the fact that he generally isn't rude or inapproprite, so I can't apologize for that. I also do feel that toward the end of this discussion some of the things I said and my choice of words were also rude and inappropriate. That I do apologize for, it was said out of frustration, but that's no excuse.

I also can't apologize for saying I was lied to. When someone tells me (and he was responding to my comment when he said it) that they are going to do something that they actually have no intention of doing, that person is lying directly to me.. so yes, I was directly lied to and I would be a liar myself if I said otherwise.

[vertis]

@stevenh512 you have no evidence that he lied though. The point made by @josevalim is completely valid.

1. It's a micro optimization
2. The optimization is tenuous at best and wouldn't make any difference to an actual app.

I was so completely wrong about @fxn, his patience must have been extremely taxed with the amount of demanding that has been going on and on over a single line of code.

[foca]

❤️ for everyone!

[tinogomes]

[radar]

@stevenh512 Just stop. Please. There's no point continuing this tirade. Nobody cares whether or not you were lied to. You are causing unnecessary stress to the core team.

[fxn]

@stevenh512 I have not lied. I am not lying. You basically have no idea what you are talking about. Enough said.

[gilesbowkett]

[parndt]

@vertis good on you :-)

@gilesbowkett lol, nice.

I was hesitant to join in on any of this but I felt like adding some positivity. The Rails core team work amazingly hard, for free, to keep Rails current and keep reviewing contributions while still driving forward to Rails 4.0 and beyond.

Thank you! I, for one, really appreciate it. Even when I do disagree it's usually over something quite trivial like code formatting :)

@josevalim for what it's worth, you're one of the best & nicest maintainers of OSS I've seen so far.. it sucks that it takes its toll on you.