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

Allow dry-running the OOBGC #5

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@FooBarWidget

FooBarWidget commented Jan 31, 2014

This is necessary for proper usage in Phusion Passenger. Passenger must send a message to its HelperAgent, and receive a message back, before it is allowed to invoke any OOBGC. This is how Passenger implements out-of-band work.

The dry-running mode allows Passenger to detect whether there is a need to run the GC. If yes, then it will request out-of-band work, and run the OOBGC in non-dry mode later. This mechanism is similar to what I described at the end of http://blog.phusion.nl/2013/01/22/phusion-passenger-4-technology-preview-out-of-band-work/

Allow dry-running the OOBGC.
This is necessary for proper usage in Phusion Passenger.
@tmm1

This comment has been minimized.

Show comment
Hide comment
@tmm1

tmm1 Jan 31, 2014

Owner

I think this will break the tuning mechanism, since it calculates the number of allocations that take place between every invocation of #run.

Owner

tmm1 commented Jan 31, 2014

I think this will break the tuning mechanism, since it calculates the number of allocations that take place between every invocation of #run.

Split version number to a version.rb.
This allows the library version to be queried.
@FooBarWidget

This comment has been minimized.

Show comment
Hide comment
@FooBarWidget

FooBarWidget Jan 31, 2014

So what would be the correct way to do it?

FooBarWidget commented Jan 31, 2014

So what would be the correct way to do it?

@tmm1

This comment has been minimized.

Show comment
Hide comment
@tmm1

tmm1 Jan 31, 2014

Owner

I'm not sure yet. Probably the thresholds should only be updated on the dry_run, but that will break backwards compat.

Is it expensive to run this after every request (always signal X-Passenger-OOB-Work)?

Owner

tmm1 commented Jan 31, 2014

I'm not sure yet. Probably the thresholds should only be updated on the dry_run, but that will break backwards compat.

Is it expensive to run this after every request (always signal X-Passenger-OOB-Work)?

@FooBarWidget

This comment has been minimized.

Show comment
Hide comment
@FooBarWidget

FooBarWidget Jan 31, 2014

Yes it is. It requires a roundtrip to another process, grabbing locks, checking states, etc.

FooBarWidget commented Jan 31, 2014

Yes it is. It requires a roundtrip to another process, grabbing locks, checking states, etc.

@tmm1

This comment has been minimized.

Show comment
Hide comment
@tmm1

tmm1 Jan 31, 2014

Owner

How about we expose the two separate parts of run() in the public api?

def OOB.run
  if OOB.checkpoint
    OOB.perform
  end
end

Then passenger can use the new methods directly.

Owner

tmm1 commented Jan 31, 2014

How about we expose the two separate parts of run() in the public api?

def OOB.run
  if OOB.checkpoint
    OOB.perform
  end
end

Then passenger can use the new methods directly.

@FooBarWidget

This comment has been minimized.

Show comment
Hide comment
@FooBarWidget

FooBarWidget Jan 31, 2014

Passenger would have to use that API in the following manner:

def on_every_request
  if OOB.checkpoint
    request_oob_work
  end
end

def on_received_oob_work_permission
  OOB.perform
end

After calling request_oob_work(), an arbitrary amount of time can pass, and an arbitrary number of requests can be handled, before on_received_oob_work_permission is called. That means that it's possible for OOB.checkpoint to be called multiple times before OOB.perform is actually called. Does that still do what you intended it to do?

FooBarWidget commented Jan 31, 2014

Passenger would have to use that API in the following manner:

def on_every_request
  if OOB.checkpoint
    request_oob_work
  end
end

def on_received_oob_work_permission
  OOB.perform
end

After calling request_oob_work(), an arbitrary amount of time can pass, and an arbitrary number of requests can be handled, before on_received_oob_work_permission is called. That means that it's possible for OOB.checkpoint to be called multiple times before OOB.perform is actually called. Does that still do what you intended it to do?

@tmm1

This comment has been minimized.

Show comment
Hide comment
@tmm1

tmm1 Jan 31, 2014

Owner

That means that it's possible for OOB.checkpoint to be called multiple times before OOB.perform is actually called.

Interesting. In this scenario is it very likely GC will already have been triggered by the time OOB.perform is actually called.

You should be able to tune for such a setup (probably by exposing the threshold constants and forcing OOB decisions much earlier.. i.e. several requests before it would be required). It's hard to say what the right tuning would be without trying some stuff in a real app and collecting data.

Owner

tmm1 commented Jan 31, 2014

That means that it's possible for OOB.checkpoint to be called multiple times before OOB.perform is actually called.

Interesting. In this scenario is it very likely GC will already have been triggered by the time OOB.perform is actually called.

You should be able to tune for such a setup (probably by exposing the threshold constants and forcing OOB decisions much earlier.. i.e. several requests before it would be required). It's hard to say what the right tuning would be without trying some stuff in a real app and collecting data.

@FooBarWidget

This comment has been minimized.

Show comment
Hide comment
@FooBarWidget

FooBarWidget Jan 31, 2014

I'm not yet concerned about optimizations. My question is, will using your API that way result in correct behavior?

FooBarWidget commented Jan 31, 2014

I'm not yet concerned about optimizations. My question is, will using your API that way result in correct behavior?

@tmm1

This comment has been minimized.

Show comment
Hide comment
@tmm1

tmm1 Jan 31, 2014

Owner

Maybe. The idea is correct but if you process more requests then GC will trigger and won't actually happen OOB. Like I said, you'd need to try it to see if it works.

Owner

tmm1 commented Jan 31, 2014

Maybe. The idea is correct but if you process more requests then GC will trigger and won't actually happen OOB. Like I said, you'd need to try it to see if it works.

@garysweaver

This comment has been minimized.

Show comment
Hide comment
@garysweaver

garysweaver Nov 26, 2014

Just curious- was there any more progress on this?

garysweaver commented Nov 26, 2014

Just curious- was there any more progress on this?

@toncid

This comment has been minimized.

Show comment
Hide comment
@toncid

toncid Dec 5, 2014

+1 any info on the progres?

toncid commented Dec 5, 2014

+1 any info on the progres?

@tmm1

This comment has been minimized.

Show comment
Hide comment
@tmm1

tmm1 Dec 5, 2014

Owner

Ruby 2.2 is around the corner and gets rid of the need to oobgc at all.

Owner

tmm1 commented Dec 5, 2014

Ruby 2.2 is around the corner and gets rid of the need to oobgc at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment