Skip to content
This repository was archived by the owner on Jun 9, 2022. It is now read-only.

Avoid caching sorted id's, just use relation.key. #1

Open
jhirn wants to merge 1 commit intodemarque:masterfrom
jhirn:master
Open

Avoid caching sorted id's, just use relation.key. #1
jhirn wants to merge 1 commit intodemarque:masterfrom
jhirn:master

Conversation

@jhirn
Copy link

@jhirn jhirn commented May 24, 2012

...ting

Hi. Because ID's are cached when the model is loaded, a reload is required after pushing objects onto the model. The ID's are already cached in the model as relation.key, so it's possible to just use that. Observe this behavior:

1.9.2p290> a.b << b2
0.0060 => [BSON::ObjectId('4fbe68488451661ad800000d')]
1.9.2p290> a.b << b
0.0035 => [BSON::ObjectId('4fbe68468451661ad8000009')]
1.9.2p290> a.b.map(&:id)
0.0001 => [BSON::ObjectId('4fbe68468451661ad8000009'), BSON::ObjectId('4fbe68488451661ad800000d')]
1.9.2p290> a.sorted_b.map(&:id)
0.0004 => [BSON::ObjectId('4fbe68468451661ad8000009'), BSON::ObjectId('4fbe68488451661ad800000d')]
1.9.2p290> a.reload 
1.9.2p290> a.sorted_b.map(&:id)
0.0019 => [BSON::ObjectId('4fbe68488451661ad800000d'), BSON::ObjectId('4fbe68468451661ad8000009')]

This commit fixes that, but you still have issue of the documents being cached after calling a.sorted_b. So this still is an issue:

1.9.2p290> a.sorted_b.map(&:id)
0.0002 => [BSON::ObjectId('4fbe68488451661ad800000d'), BSON::ObjectId('4fbe68468451661ad8000009')]
1.9.2p290> a.sorted_b << o3
0.0021 => [BSON::ObjectId('4fbe6a158451661ad8000011')]
1.9.2p290> a.sorted_b.map(&:id)
0.0001 => [BSON::ObjectId('4fbe68488451661ad800000d'), BSON::ObjectId('4fbe68468451661ad8000009')]

I understand a desire for caching here and would like to discuss the need for it vs making it optional, but this pull request can be merged in regardless to eliminate the unnecessary cache object. I can open a separate issue for the latter.

@srosa
Copy link
Contributor

srosa commented May 28, 2012

Hi,

First of all, thanks for the fix and the test!

But before I merge let me explain why the cache is there (apart of the performance gain). On our current version of Mongoid (2.0.2 we cannot upgrade yet until 3.0 is out), if we call the relation without the sorted_ prefix, mongoid will set the array of ids back to the "natural order". See the example below :

b = Books.first
# => #<Book _id: 4fb16167942179a62000007d, author_ids: [BSON::ObjectId('4fa2b9ab9421791024000010'), BSON::ObjectId('4f9161829421797635000142'), BSON::ObjectId('4f916182942179763500012c')]> 
b.authors.count
# => 10 
b.author_ids
# => [BSON::ObjectId('4f916182942179763500012c'), BSON::ObjectId('4f9161829421797635000142'), BSON::ObjectId('4fa2b9ab9421791024000010')]

Worst, if the model is saved afterward, the order is completely lost. The cache is an attempt to protect us from this behavior. I don't know yet if this is fix in future release of Mongoid but for now, it's useful to us.

I think your suggestion of making it optional is great and we will surely implement that in a near future.

I'll open a new issue right away ...

@jhirn
Copy link
Author

jhirn commented May 30, 2012

Ah I see. We are using 2.4.10 of mongoid and it seems to behave a bit better.

Well thanks for this gem. I literally needed it the day after you published it. I will keep an eye on this and help out where I can.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants