sabre-io/http

Question concering Request class

jens1o opened this issue · 5 comments

jens1o commented

Hi there,

first of all, I love using this library, but I have one question:

https://github.com/fruux/sabre-http/blob/7d91cdb1417eddbfa5e433b298345ad67d4f9266/lib/Request.php#L178-L202

Why don't you cache the result? String operations are really slow in php... That is something that bothers and worries me... If you mind changing it, I'll create a simple pr.

Regards and thanks

staabm commented

In which case is this method slow and caching is worth it (example code)?

jens1o commented

When you use it for routing, but get the path twice because of MVC controllers which have access to the Request class and use it again and again. So after all, I call this method at least 4 times in my applications, but I don't want to build a wrapper for such a simple mechanism and maybe this improves performance of other websites, too.

I use this library in time-critical applications, for example in the web, api requests for other servers where users issue commands and I deliver what they need to proceed that data.

I think this line is pretty slow: https://github.com/fruux/sabre-http/blob/7d91cdb1417eddbfa5e433b298345ad67d4f9266/lib/Request.php#L190
And doing this multiple times is not worth it. Unfortunately, I can't provide some source code because I'm not allowed to do that. If you have any additional question, don't hesitate to ask.

staabm commented

please profile your app and show us those 4 calls cost a lot of time.

a cache is only worth it in case we save something. just calling a method several times, but still dont take much time for it is not argument.

this method takes other properties into account which can change during object lifetime, so caching the result of getPath() would als require invalidation logic in some places..

jens1o commented

Okay, you'd won:

<?php
require_once './../vendor/autoload.php';

$request = new Sabre\HTTP\Request('POST', '/test');

$request->setBaseUrl('/');

$start = microtime(true);

$request->getPath();
$request->getPath();
$request->getPath();
$request->getPath();

$end = microtime(true);

echo 'Took ' . round($end - $start, 4) . 'ms';

Comes up with Took 0.0008ms...

staabm commented

thx for checking and thx for your contribution nevertheless!

  • 👍1