Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

fixed AbstractAdapter::internalDecrementItems #36

Merged
merged 1 commit into from
Dec 20, 2015
Merged

fixed AbstractAdapter::internalDecrementItems #36

merged 1 commit into from
Dec 20, 2015

Conversation

marc-mabe
Copy link
Member

  • was calling decrementItem instead of internalDecrementItem
  • triggered events for bulk decrement as well as for every items

Noted by benchmarks of #35

Before:

ZendBench\Cache\MemoryStorageAdapterBench
    Method Name                    Iterations   Average Time      Ops/second 
    ----------------------------- ------------ ----------------- -------------
    decrementMissingItemsSingle : [       100] [0.0008270716667] [1,209.08507]
    decrementMissingItemsBulk   : [       100] [0.0009380531311] [1,066.03770]
    decrementExistingItemsSingle: [       100] [0.0008216524124] [1,217.05965]
    decrementExistingItemsBulk  : [       100] [0.0009412384033] [1,062.43009]

After:

ZendBench\Cache\MemoryStorageAdapterBench
    Method Name                    Iterations   Average Time      Ops/second 
    ----------------------------- ------------ ----------------- -------------
    decrementMissingItemsSingle : [       100] [0.0008160185814] [1,225.46229]
    decrementMissingItemsBulk   : [       100] [0.0001660728455] [6,021.45400]
    decrementExistingItemsSingle: [       100] [0.0008193802834] [1,220.43454]
    decrementExistingItemsBulk  : [       100] [0.0001683640480] [5,939.51032]

PS: tests are passing but I need to write a new test catching this

 * was calling decrementItem instead of internalDecrementItem
 * triggered events for buld decrement as well as for every items
@marc-mabe marc-mabe added this to the 2.5.4 milestone Nov 4, 2015
@marc-mabe marc-mabe removed the WIP label Nov 10, 2015
@marc-mabe marc-mabe self-assigned this Nov 10, 2015
@marc-mabe
Copy link
Member Author

Test is added by #38

@Maks3w
Copy link
Member

Maks3w commented Nov 11, 2015

I prefer if tests are added in this PR. #38 is too long

@marc-mabe
Copy link
Member Author

@Maks3w

I prefer if tests are added in this PR. #38 is too long

-> If I merge #38 into this one it gets more long.

Does I understand you correctly that you prefer to only move the one failing test into this PR?

I would more prefer to ignore this PR and see #38 as only one thing that adds missing tests to AbstractAdapter as is where one test fails. Afterwards this PR comes up to fix this bug.
-> Nearly all the tests in #38 are written in a very similar way including testGetMetadatas[Fail], testSetItems[Fail], testAddItems[Fail|Exists], testReplaceItems[Fail|Missing], testRemoveItems[Fail], testIncrementItems[Fail], testDecrementItems[Fail], testTouchItems[Fail]. It would be strange to move testIncrementItems into another PR.

@marc-mabe
Copy link
Member Author

@Maks3w ping

@marc-mabe marc-mabe merged commit 8ef2db4 into zendframework:master Dec 20, 2015
marc-mabe added a commit that referenced this pull request Dec 20, 2015
…DecrementItems

fixed AbstractAdapter::internalDecrementItems
marc-mabe added a commit that referenced this pull request Dec 20, 2015
marc-mabe added a commit that referenced this pull request Dec 20, 2015
@marc-mabe marc-mabe deleted the hotfix/AbstractAdapter_internalDecrementItems branch December 20, 2015 20:31
@marc-mabe marc-mabe modified the milestones: 2.6.0, 2.5.4 Feb 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants