-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New PR for the cache function #7
Closed
Closed
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
dfa86d6
Redid the PR based on the comments: added caching (optional) to the C…
eaedd27
Comment changes
RonRademaker 3afc3e5
Moved speed test to POC script as it's not 100% reliable for unit tes…
RonRademaker 63d85b8
Fixed whitespace in comment, removed incorrect api statement, added s…
RonRademaker 73110b7
Updated file name
RonRademaker 4b935b0
Added strict boolean check, removed error supression
RonRademaker 5c752bb
Ran CS fixer
RonRademaker File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<test> | ||
<!-- Configuration for unit testing --> | ||
<foo bar='bar attribute'> | ||
<foo bar='more bar attribute'/> | ||
</foo> | ||
<bar> | ||
<fuzz id='fuzzy'/> | ||
<fuzz id='very fuzzy'/> | ||
</bar> | ||
<fuzzy>Fuzzy text content</fuzzy> | ||
</test> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
<?php | ||
|
||
use Nijens\Utilities\Configuration; | ||
|
||
/* | ||
* POC script to demonstrate caching is usually faster | ||
* As it's not always faster, it's not suitable to be tested in a unit test | ||
*/ | ||
require '../vendor/autoload.php'; | ||
|
||
foreach ([2, 3] as $count) { | ||
$success = 0; | ||
for ($i = 0; $i < 100; ++$i) { | ||
$cached = new Configuration(__DIR__ . '/Resources/configuration/default.xml', __DIR__ . '/Resources/xsd/default.xsd'); | ||
$cached->loadConfiguration(null); | ||
$notCached = new Configuration(__DIR__ . '/Resources/configuration/default.xml', __DIR__ . '/Resources/xsd/default.xsd', false); | ||
$notCached->loadConfiguration(null); | ||
$cachedStart = microtime(true); | ||
for ($j = 0; $j < $count; ++$j) { | ||
$cached->get('/test/foo'); | ||
} | ||
$cachedTime = microtime(true) - $cachedStart; | ||
|
||
$notCachedStart = microtime(true); | ||
for ($j = 0; $j < $count; ++$j) { | ||
$notCached->get('/test/foo'); | ||
} | ||
$notCachedTime = microtime(true) - $notCachedStart; | ||
|
||
if ($notCachedTime > $cachedTime) { | ||
++$success; | ||
} | ||
} | ||
echo "Cached was fastest {$success} times out of 100 on {$count} calls\n"; | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach can be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it can but it'd change that the scripts tests. You could add a 4 and a 5 (or others) to the array as well, just depends on what you want to script to test. I choose to test the performance benefits when calling the same get 2 or 3 times, obviously you'd benefit more from caching as you call a function that can return a cached variable more often.