ktamas77 / firebase-php

Firebase PHP Client
791 stars 215 forks source link

Why not take advantage of Keep-Alive? #14

Closed kijin closed 9 years ago

kijin commented 10 years ago

firebase-php promptly closes the curl handle after each request, making it impossible to take advantage of the underlying protocol's Keep-Alive capability. Since all Firebase API requests use SSL, using Keep-Alive would save a massive amount of time and resources (SSL handshake in addition to TCP handshake) when multiple requests are made in quick succession.

Enabling Keep-Alive with PHP curl is as simple as reusing the same curl handle without closing it.

BatuhanK commented 10 years ago

Yes, please. It's really bad to use Firebase in this way.

ktamas77 commented 10 years ago

it's relatively easy to implement, please feel free to fork the repository and send a pull request if you want to have this functionality immediately. otherwise i'll examine the best possible implementation soon. thank you for your suggestion.

BatuhanK commented 10 years ago

@ktamas77 Too busy with midterms :-1:

Waiting for your solution :) Thank you for fast response

kijin commented 10 years ago

Just opened a pull request that adds Keep-Alive support. Please test. Thanks.

BatuhanK commented 10 years ago

I added your method + fsocks multi set method. https://github.com/ktamas77/firebase-php/pull/16

And good for news who needs amazing write speeds !

  • Added parallel curl with shell_exec; only works in linux ( i guess ). But you have no control after setting data, no callbacks no tracing etc.

Some stats (on digitalocean vps - ubuntu 12.04 )

kijin commented 10 years ago

Amazing speed is great and all, but I'm not sure whether it should be the job of an API client library to handle parallelization through platform-specific shell commands. Parallelization should be the job of a different library, of which PHP already has plenty.

Not to mention this opens the door to all sorts of nasty security bugs. (addslashes? seriously?)

The fsockopen version looks cool, though. Could you add Keep-Alive support to that? It seems wasteful to close the socket after every request. As long as you don't need to read the responses, it should be just as fast as, if not faster than, the shell_exec method. Forking a shell is kinda expensive.

Anyway, just my $0.02 as the original opener of this issue. Of course @ktamas77 will have the final say.

BatuhanK commented 10 years ago

@kijin haha you're right :)

in push request i mentioned :

Be careful it contains shell_exec method and json_data ( may contains your user inputs ). so if you are not trusting to data, I suggest you to sanitize it carefully.

What is 0.02$, i don't understand.

(i'm newbie in version controlling and github. Sorry if I made mistake )

kijin commented 10 years ago

What is 0.02$, i don't understand.

Haha, it's just an American idiom, "my two cents".

BatuhanK commented 10 years ago

@kijin Dude, i just added keep-alive support for fsocks and removed shell_exec method. https://github.com/ktamas77/firebase-php/pull/16

You have to try it :+1: IT'S AWESOME !!

ktamas77 commented 9 years ago

I agree, it's a nice improvement, please see my comment on https://github.com/ktamas77/firebase-php/pull/15 and https://github.com/ktamas77/firebase-php/pull/16