unused data

[hyperloop-rails]

lobsters/app/models/vote.rb

Lines 70 to 75 in 8db5de9

	votes = self.where(
	:user_id => user_id,
	:comment_id => comment_ids,
	)
	votes.inject({}) do |memo, v|
	memo[v.comment_id] = { :vote => v.vote, :reason => v.reason }

There is unused data retrieval, only comment_id, vote, reason of Vote is used, so the code can be modified to:
votes = self.where( :user_id => user_id, :comment_id => comment_ids, ).select(:comment_id, vote, reason)

[pushcx]

This is not the correct refactoring of this code. Zoom out more.

[hyperloop-rails]

Here is the patch

diff --git a/app/models/vote.rb b/app/models/vote.rb
index 4e78e74..e96ac38 100644
--- a/app/models/vote.rb
+++ b/app/models/vote.rb
@@ -72,7 +72,7 @@ class Vote < ActiveRecord::Base
       votes = self.where(
         :user_id    => user_id,
         :comment_id => comment_ids,
-      )
+      ).select(:comment_id, :vote, :reason)
       votes.inject({}) do |memo, v|
         memo[v.comment_id] = { :vote => v.vote, :reason => v.reason }
         memo

[pushcx]

This is still not the correct refactoring of the code. I did understand what you suggested but this microoptimization is not the right fix. I was being terse because you sound like a bot or a service expecting free beta testing.

Your paper turned up on Lobsters, that was useful context for this and #518: https://lobste.rs/s/u7srss/how_not_structure_your_database_backed#c_syfieu If you'd like invites to join the conversation, send me an email or DM on twitter. Or if you'd like to grab coffee, I'm on the south side most Tuesdays.

[pushcx]

@hyperloop-rails It's been a couple weeks - anyone there?

[hyperloop-rails]

First of all, could you point out which part of the patch is wrong. Since I test the function and the output is the same.
Secondly, Actually we generate this patch is to eliminate the unnecessary data retrieval, as in this example, not used fields are those other than vote, comment_id, reason, so we only retrieve them instead of all fields. In this particular example, I admit the effort is not large enough due to the unused fields didn't occupy that much space. But what we really want to solve is the general cases.
Also, this kind of unnecessary data retrieval should be handled by ORM level instead of developers, which is more efficient and makes the code more readable.

[pushcx]

The patch isn't wrong. The patch looks like a bot outputting rules-based fixes rather than a human reading the code, because there's a better solution to this if you look at this function, and another better one if you look at the larger context in the app.

So if you have a bot, great, let's talk about your bot on the site and decide what would be useful. But running the bot and dropping micro-optimizations means you're volunteering me to be the human to spend time thinking about them.

I'm really happy to see the codebase was useful in your studies, because the time contributors spent on it is providing bonus value at no extra work. When you paste bot output and use that as a metric in your next paper as evidence of your bot's value, you're asking contributors to do new work for you. When it's micro-optimizations with no human judgement or even performance metrics, you're providing an extremely marginal benefit to the codebase that is not outweighed by the volunteer time required to do review it.

The problem is not that I don't understand what your patch does or what benefits you claim or what the general class of performance issue is. The problem is that you're not acting like you care about the costs.

[pushcx]

Closed. I'm out of polite things to say about you trying to use me like this.

[hyperloop-rails]

I feel so sorry to make you feel bad, and I appreciate a lot for your effort in responding to the issues I posted.

Indeed we have a recently published tool (a RubyMine plugin) to help you identify such optimization and do automatic fix, as you can find here: https://hyperloop-rails.github.io/powerstation/, which you can try it on your own Rails project.

I'm not intended to make you feel that I'm using you--I apologize sincerely to hurt feelings--but want to improve the performance problem solving in ORM applications. Helping find out both performance-improving and semantics doable patches is one of our target. And your feedback about the costs concern really helps us a lot in our future work of better tools.

Anyway, I apologize again for my mis-behavior.

[pushcx]

You didn't hurt my feelings, you repeatedly neglected to identify yourself as researchers so that professional work me and other maintainers into participating in and evaluating your research project without consent. My feelings aren't hurt by these code patches. For the final time, the problem is your misconduct as an academic researcher.