Clean up: Remove GlobalHistogramAllocator::CreateWithSharedMemory

Clean up: Remove GlobalHistogramAllocator::CreateWithSharedMemory

That function is not used anywhere. This CL removes that for code
health.

BUG=n/a
TEST=n/a

Review-Url: https://codereview.chromium.org/2715543005
Cr-Commit-Position: refs/heads/master@{#458674}
Committed: https://chromium.googlesource.com/chromium/src/+/232d4bc8809a76c118af5aed8aa824b1795eab25

[hajimehoshi, 2017-02-24 09:37:46]

The CQ bit was checked by hajimehoshi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 09:37:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hajimehoshi, 2017-02-24 09:38:08]

hajimehoshi@chromium.org changed reviewers:
+ isherman@chromium.org

[hajimehoshi, 2017-02-24 09:38:09]

PTAL

[hajimehoshi, 2017-02-24 09:38:32]

Description was changed from

==========
Clean up: Remove GlobalHistogramAllocator::CreateWithSharedMemory

That function is not used anywhere. This CL removes this for code
health.

BUG=n/a
TEST=n/a
==========

to

==========
Clean up: Remove GlobalHistogramAllocator::CreateWithSharedMemory

That function is not used anywhere. This CL removes that for code
health.

BUG=n/a
TEST=n/a
==========

[commit-bot: I haz the power, 2017-02-24 10:30:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 10:30:41]

Dry run: This issue passed the CQ dry run.

[Alexei Svitkine (slow), 2017-02-24 16:39:28]

asvitkine@chromium.org changed reviewers:
+ bcwhite@chromium.org

[Alexei Svitkine (slow), 2017-02-24 16:39:29]

+bcwhite

[bcwhite, 2017-02-24 18:47:04]

On 2017/02/24 16:39:29, Alexei Svitkine (slow) wrote:
> +bcwhite

How about this instead?
https://codereview.chromium.org/2714963002

[Ilya Sherman, 2017-02-24 21:36:05]

On 2017/02/24 18:47:04, bcwhite wrote:
> On 2017/02/24 16:39:29, Alexei Svitkine (slow) wrote:
> > +bcwhite
> 
> How about this instead?
> https://codereview.chromium.org/2714963002

What's the advantage of sharing the code between a used codepath and an unused
one, rather than just removing the unused one?

[hajimehoshi, 2017-03-01 09:30:52]

On 2017/02/24 21:36:05, Ilya Sherman wrote:
> On 2017/02/24 18:47:04, bcwhite wrote:
> > On 2017/02/24 16:39:29, Alexei Svitkine (slow) wrote:
> > > +bcwhite
> > 
> > How about this instead?
> > https://codereview.chromium.org/2714963002
> 
> What's the advantage of sharing the code between a used codepath and an unused
> one, rather than just removing the unused one?

ping bcwhite@

[chromium-reviews, 2017-03-01 15:38:38]

He's away this week.

On Wed, Mar 1, 2017 at 4:30 AM, <hajimehoshi@chromium.org> wrote:

> On 2017/02/24 21:36:05, Ilya Sherman wrote:
> > On 2017/02/24 18:47:04, bcwhite wrote:
> > > On 2017/02/24 16:39:29, Alexei Svitkine (slow) wrote:
> > > > +bcwhite
> > >
> > > How about this instead?
> > > https://codereview.chromium.org/2714963002
> >
> > What's the advantage of sharing the code between a used codepath and an
> unused
> > one, rather than just removing the unused one?
>
> ping bcwhite@
>
> https://codereview.chromium.org/2715543005/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[bcwhite, 2017-03-06 13:48:53]

On 2017/02/24 21:36:05, Ilya Sherman wrote:
> On 2017/02/24 18:47:04, bcwhite wrote:
> > On 2017/02/24 16:39:29, Alexei Svitkine (slow) wrote:
> > > +bcwhite
> > 
> > How about this instead?
> > https://codereview.chromium.org/2714963002
> 
> What's the advantage of sharing the code between a used codepath and an unused
> one, rather than just removing the unused one?

Completeness.  There was a time when that method was used and it's possible
there will be again.  It's actually the logical entry point with the other being
added as a convenience method to avoid code duplication outside.

This should have the same reduced code size and keep all the code exercised but
still provide the additional entry point.

[Ilya Sherman, 2017-03-06 19:03:17]

On 2017/03/06 13:48:53, bcwhite wrote:
> On 2017/02/24 21:36:05, Ilya Sherman wrote:
> > On 2017/02/24 18:47:04, bcwhite wrote:
> > > On 2017/02/24 16:39:29, Alexei Svitkine (slow) wrote:
> > > > +bcwhite
> > > 
> > > How about this instead?
> > > https://codereview.chromium.org/2714963002
> > 
> > What's the advantage of sharing the code between a used codepath and an
unused
> > one, rather than just removing the unused one?
> 
> Completeness.  There was a time when that method was used and it's possible
> there will be again.  It's actually the logical entry point with the other
being
> added as a convenience method to avoid code duplication outside.
> 
> This should have the same reduced code size and keep all the code exercised
but
> still provide the additional entry point.

Hmm, I see where you're coming from, and I'm not really convinced -- if we kept
all of the "might be useful someday" code around, we'd have a lot of extra code
to maintain.  That said, I left a suggestion on your proposed CL if you'd really
prefer to split this method into two pieces.

[bcwhite, 2017-03-10 18:48:42]

LGTM

> Hmm, I see where you're coming from, and I'm not really convinced -- if we
kept
> all of the "might be useful someday" code around, we'd have a lot of extra
code
> to maintain.  That said, I left a suggestion on your proposed CL if you'd
really
> prefer to split this method into two pieces.

I think keeping it split and moving 1/2 of it to an anonymous namespace (the
comment in the other CL) would make it confusing.  Forget it.  I'll recreate it
if it's ever necessary.

[hajimehoshi, 2017-03-22 05:47:12]

The CQ bit was checked by hajimehoshi@chromium.org

[commit-bot: I haz the power, 2017-03-22 05:47:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 05:54:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-22 05:54:18]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ilya Sherman, 2017-03-22 06:41:11]

LGTM

[Ilya Sherman, 2017-03-22 06:41:20]

The CQ bit was checked by isherman@chromium.org

[commit-bot: I haz the power, 2017-03-22 06:41:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 07:47:50]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1490164880026940, "parent_rev":
"8452ddd64771d3573cd7083f3e6e263cf67fbe1b", "commit_rev":
"232d4bc8809a76c118af5aed8aa824b1795eab25"}

[commit-bot: I haz the power, 2017-03-22 07:48:32]

Description was changed from

==========
Clean up: Remove GlobalHistogramAllocator::CreateWithSharedMemory

That function is not used anywhere. This CL removes that for code
health.

BUG=n/a
TEST=n/a
==========

to

==========
Clean up: Remove GlobalHistogramAllocator::CreateWithSharedMemory

That function is not used anywhere. This CL removes that for code
health.

BUG=n/a
TEST=n/a

Review-Url: https://codereview.chromium.org/2715543005
Cr-Commit-Position: refs/heads/master@{#458674}
Committed:
https://chromium.googlesource.com/chromium/src/+/232d4bc8809a76c118af5aed8aa8...
==========

[commit-bot: I haz the power, 2017-03-22 07:48:33]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/232d4bc8809a76c118af5aed8aa8...