Skip to content
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

Why not take advantage of Keep-Alive? #14

Closed
kijin opened this issue Oct 8, 2014 · 10 comments
Closed

Why not take advantage of Keep-Alive? #14

kijin opened this issue Oct 8, 2014 · 10 comments

Comments

@kijin
Copy link

kijin commented Oct 8, 2014

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
Copy link

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

@ktamas77
Copy link
Owner

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
Copy link

@ktamas77 Too busy with midterms 👎

Waiting for your solution :) Thank you for fast response

kijin added a commit to kijin/firebase-php that referenced this issue Oct 28, 2014
@kijin
Copy link
Author

kijin commented Oct 28, 2014

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

@BatuhanK
Copy link

I added your method + fsocks multi set method. #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 )

  • [default] 50 set request with set method [ it using cURL ]=> 30sec
  • [mine]50 set requests with set_multi method [ it using fsocks ] => 12 sec
  • [mine]50 set requests with set_fast method [ it using cURL with shell exec /
    parallel ] => 0.4 sec

@kijin
Copy link
Author

kijin commented Oct 28, 2014

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
Copy link

@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
Copy link
Author

kijin commented Oct 28, 2014

What is 0.02$, i don't understand.

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

@BatuhanK
Copy link

@kijin
Dude, i just added keep-alive support for fsocks and removed shell_exec method. #16

You have to try it 👍 IT'S AWESOME !!

@ktamas77
Copy link
Owner

ktamas77 commented May 8, 2015

I agree, it's a nice improvement, please see my comment on #15 and #16

@ktamas77 ktamas77 closed this as completed May 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants