ChannelMojo: Grow IPC message upper limit to 128MB

ChannelMojo: Grow IPC message upper limit to 128MB

There are assumptions that the IPC message limit is 128MB
so ChannelMojo has to allow messages in upto the message size.
Such large messages happen when IDB reads/writes huge records.

BUG=464151
R=agl@chromium.org

Committed: https://crrev.com/5690575b71ebeb8e3bdb5ecdbadb8874be3f3f50
Cr-Commit-Position: refs/heads/master@{#322072}

[darin (slow to review), 2015-03-24 18:07:29]

Wow, seems like there is an opportunity to optimize IndexedDB IPC. Perhaps
once we are using Mojo IPC for it, we can use a DataPipe for transferring
(streaming) such huge records.

On Mon, Mar 23, 2015 at 2:12 PM, <morrita@chromium.org> wrote:

> Reviewers: agl,
>
> Description:
> ChannelMojo: Grow IPC message upper limit to 128MB
>
> There are assumptions that the IPC message limit is 128MB
> so ChannelMojo has to allow messages in upto the message size.
> Such large messages happen when IDB reads/writes huge records.
>
> BUG=464151
> R=agl@chromium.org
>
> Please review this at https://codereview.chromium.org/1029703002/
>
> Base URL: https://chromium.googlesource.com/chromium/src.git@master
>
> Affected files (+1, -1 lines):
>   M content/app/mojo/mojo_init.cc
>
>
> Index: content/app/mojo/mojo_init.cc
> diff --git a/content/app/mojo/mojo_init.cc b/content/app/mojo/mojo_init.cc
> index c70625552bb9b77124eae1fb941e2cdcfac98395..
> 4b4af02a54f4ef13d80742c160413dba3804efcf 100644
> --- a/content/app/mojo/mojo_init.cc
> +++ b/content/app/mojo/mojo_init.cc
> @@ -18,7 +18,7 @@ class MojoInitializer {
>   public:
>    MojoInitializer() {
>      mojo::embedder::GetConfiguration()->max_message_num_bytes =
> -        64 * 1024 * 1024;
> +        128 * 1024 * 1024;
>      mojo::embedder::Init(scoped_ptr<mojo::embedder::PlatformSupport>(
>          new mojo::embedder::SimplePlatformSupport()));
>    }
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jsbell, 2015-03-24 18:10:28]

Note that a lower limit is acceptable too, we just need to ensure that code that
inquires about the maximum IPC message size gets the right answer so that we can
preemptively reject requests that will fail:

https://code.google.com/p/chromium/codesearch#chromium/src/content/child/inde...

... but also no objection to bumping to 128MB to match the pre-Mojo limit.

[agl, 2015-03-24 18:17:03]

I'm not sure that I can usefully opine about this, but if knowledge people think
it's the right thing to do (for now, at least) then LGTM.

[Hajime Morrita, 2015-03-24 19:51:19]

On 2015/03/24 18:10:28, jsbell wrote:
> Note that a lower limit is acceptable too, we just need to ensure that code
that
> inquires about the maximum IPC message size gets the right answer so that we
can
> preemptively reject requests that will fail:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/child/inde...
> 
> ... but also no objection to bumping to 128MB to match the pre-Mojo limit.

Ouch, I should use the constant on IPC::Channel instead of hard coding the
number. Will update before landing.

[Hajime Morrita, 2015-03-24 19:52:38]

The CQ bit was checked by morrita@chromium.org

[Hajime Morrita, 2015-03-24 19:52:38]

The patchset sent to the CQ was uploaded after l-g-t-m from agl@chromium.org
Link to the patchset: https://codereview.chromium.org/1029703002/#ps20001
(title: "Landing")

[commit-bot: I haz the power, 2015-03-24 19:53:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1029703002/20001

[commit-bot: I haz the power, 2015-03-24 21:21:12]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-24 21:21:51]

Patchset 2 (id:??) landed as
https://crrev.com/5690575b71ebeb8e3bdb5ecdbadb8874be3f3f50
Cr-Commit-Position: refs/heads/master@{#322072}

[jsbell, 2015-03-24 23:26:46]

On 2015/03/24 18:07:29, darin (slow to review) wrote:
> Wow, seems like there is an opportunity to optimize IndexedDB IPC. Perhaps
> once we are using Mojo IPC for it, we can use a DataPipe for transferring
> (streaming) such huge records.

Yep. :)

Fortunately, this is pretty rare. Now that we have Blob support in IDB, we
encourage developers to use that for big records. dmurph@ is working on scaling
up the blob transport system to handle ridiculously large amounts of data too.
We've also considered auto-blobifying big IDB records anyway, since leveldb
doesn't handle big records particularly well either.