Closed (outdated)
Project:
Drupal core
Version:
11.x-dev
Component:
other
Priority:
Normal
Category:
Task
Assigned:
Issue tags:
Reporter:
Created:
23 Jun 2010 at 13:10 UTC
Updated:
22 Apr 2025 at 18:32 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
alippai commentedNot tested, not benchmarked.
Comment #2
alippai commentedComment #4
alippai commentedComment #6
alippai commentedComment #7
alippai commentedComment #8
alippai commentedMemory benchmark (in bytes):
- means not executed in 300 seconds, with patch last executed in cca 10.1 seconds.
The testcode:
AB benchmark coming tomorrow.
Comment #9
chx commentedWow! Very interesting nice work. However, claimItem should put the queue key in the item so that release and delete can reuse that.
Comment #10
yched commentedIsn't there a smarter way than looping through the whole queue to find the item ?
At the very least, it looks like the loop should break once the item was found ?
+ missing whitespace after 'for' and 'if'.
57 critical left. Go review some!
Comment #11
yched commentedHm, also, it looks like this API is only available in PHP 5.3, which would be a no go for drupal core.
Comment #12
alippai commentedThere is prettier way to iterate, but we have to iterate thru it.
SPL is available and compiled by default in PHP 5.0.0 and can't be disabled from version 5.3.0.
If we use it, it should be added to the Drupal requirements.
Go or no go? Go only for D8?
Comment #13
alippai commentedSPL is already required - system.install, line 117
I'm going to continue my work now.
P.S.: Thanks to catch for pointing this out.
Comment #14
alippai commentedWho calls releaseItem()? If the "claimer" has to, in that case he doesn't know the expiration time in the SystemQueue (maybe a bug?). Should be there a scheduler for every queue?
Comment #15
chx commentedYou are not getting me. You should not iterate your queue when deleting / releasing. Put the index in claimItem into $item: $item = $this->queue->current(); $item->key = $this->queue->key(); return $item reuse it in deleteItem: $this->queue->offsetUnset($item->key);
Also SplQueue extends SplDoublyLinkedList implements Iterator , ArrayAccess please use array notation, isnt that simpler? But also please dont forget to re-benchmark to do it. ($queue = $this->queue; unset($queue[$item->key]);)
As for releaseItem and expire, I dont get your question. You are leasing the item for say 30 seconds. If you are not done in 30 seconds, another process might get the chance to process the item. If you are done, just call deleteItem. If you bail out early because say you wanted to reach an external resource and there is a network error then call releaseItem so that another process can try again. You do not need a scheduler for any of these.
Comment #16
alippai commentedSome work on this.
Comment #18
alippai commentedLeft there $this->...
Comment #19
alippai commentedComment #21
alippai commentedWith this patch there is no difference in memory load and execution time (with or without the patch), i have to figure this out later.
Comment #22
mikedillion commented#14: 835672-14-memoryqueue.patch queued for re-testing.
Comment #23
catchMoving this to 8. Looks like a candidate for backport though if the previous numbers hold up.
Comment #24
catchComment #25
alippai commentedOkay, this patch may be oversimplified, but the results are nice:
This patch changes the behavior. When you claim an item, you remove it from the queue. When you release the item, you put it back. This way it doesn't have a item_id (and there is no need for that).
I don't know about it's usage in the core, but the tests are green. :P
Comment #26
alippai commentedAlso we should update the BatchMemory implementation.
About the stdClass() wrapper for the data: it's needed to match the "interface", but we don't use it now in this implementation.
Comment #28
alippai commented#25: d8-835672-25-memoryqueue.patch queued for re-testing.
Comment #30
alippai commentedFixed BatchMemory.
Comment #31
alippai commentedComment #33
damiankloip commented#30: d8-835672-30-memoryqueue.patch queued for re-testing.
Comment #49
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #50
smustgrave commentedSince there has been no follow up in 3 months going to close out. But don't worry they can always be re-opened.
Thanks all!