LinkedStack.h is no longer used, remove it

LinkedStack.h is no longer used, remove it

BUG=none
NO_DEPENDENCY_CHECKS=true

Review-Url: https://codereview.chromium.org/2761853003
Cr-Commit-Position: refs/heads/master@{#458376}
Committed: https://chromium.googlesource.com/chromium/src/+/951df7be3b2b83f682351f98300354f7a2807be9

[Mostyn Bramley-Moore, 2017-03-20 20:36:57]

The CQ bit was checked by mostynb@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-20 20:37:26]

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

[Mostyn Bramley-Moore, 2017-03-20 20:42:28]

Description was changed from

==========
LinkedStack.h is no longer used, remove it
==========

to

==========
LinkedStack.h is no longer used, remove it

BUG=none
==========

[Mostyn Bramley-Moore, 2017-03-20 20:42:28]

mostynb@opera.com changed reviewers:
+ haraken@chromium.org, sigbjornf@opera.com

[Mostyn Bramley-Moore, 2017-03-20 20:54:06]

@haraken: please take a look at this dead code removal.

[dglazkov, 2017-03-20 20:57:26]

dglazkov@chromium.org changed reviewers:
+ dglazkov@chromium.org

[dglazkov, 2017-03-20 20:57:27]

lgtm

[sof, 2017-03-20 20:57:31]

sigbjornf@opera.com changed reviewers:
- dglazkov@chromium.org

[sof, 2017-03-20 20:57:31]

Keep HeapLinkedStack<>, but not LinkedStack<> ?

[Mostyn Bramley-Moore, 2017-03-20 20:58:51]

On 2017/03/20 20:57:31, sof wrote:
> Keep HeapLinkedStack<>, but not LinkedStack<> ?

HeapLinkedStack is still used in RuleSet.cpp and HeapTest.cpp.

[Mostyn Bramley-Moore, 2017-03-20 20:59:02]

The CQ bit was unchecked by mostynb@opera.com

[Mostyn Bramley-Moore, 2017-03-20 20:59:11]

The CQ bit was checked by mostynb@opera.com

[commit-bot: I haz the power, 2017-03-20 20:59:49]

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

[commit-bot: I haz the power, 2017-03-20 20:59:50]

This CL has an open dependency (Issue 1505823003 Patch 120001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[sof, 2017-03-20 21:00:36]

On 2017/03/20 20:58:51, Mostyn Bramley-Moore wrote:
> On 2017/03/20 20:57:31, sof wrote:
> > Keep HeapLinkedStack<>, but not LinkedStack<> ?
> 
> HeapLinkedStack is still used in RuleSet.cpp and HeapTest.cpp.

Yes, so it makes some sense to keep a symmetric set (Heap and not) around.

[Mostyn Bramley-Moore, 2017-03-20 23:04:15]

On 2017/03/20 21:00:36, sof wrote:
> > > Keep HeapLinkedStack<>, but not LinkedStack<> ?
> > 
> > HeapLinkedStack is still used in RuleSet.cpp and HeapTest.cpp.
> 
> Yes, so it makes some sense to keep a symmetric set (Heap and not) around.

Even if LinkedStack is no longer tested (beyond compilation testing)?

It looks like HeapLinkedStack is currently only used as intermediate storage
before compacting them into HeapTerminatedArray's.  I could investigate cutting
out the middleman, if you think it's worthwhile (and can help me figure out how
to mem + perf test such a change).  If not, I can at least add some
documentation to these classes.

[haraken, 2017-03-20 23:33:54]

On 2017/03/20 23:04:15, Mostyn Bramley-Moore wrote:
> On 2017/03/20 21:00:36, sof wrote:
> > > > Keep HeapLinkedStack<>, but not LinkedStack<> ?
> > > 
> > > HeapLinkedStack is still used in RuleSet.cpp and HeapTest.cpp.
> > 
> > Yes, so it makes some sense to keep a symmetric set (Heap and not) around.
> 
> Even if LinkedStack is no longer tested (beyond compilation testing)?
> 
> It looks like HeapLinkedStack is currently only used as intermediate storage
> before compacting them into HeapTerminatedArray's.  I could investigate
cutting
> out the middleman, if you think it's worthwhile (and can help me figure out
how
> to mem + perf test such a change).  If not, I can at least add some
> documentation to these classes.

+1 to removing HeapLinkedStack. Reducing # of data structures in the code base
sounds nice.

[sof, 2017-03-21 06:36:53]

On 2017/03/20 23:33:54, haraken wrote:
> On 2017/03/20 23:04:15, Mostyn Bramley-Moore wrote:
> > On 2017/03/20 21:00:36, sof wrote:
> > > > > Keep HeapLinkedStack<>, but not LinkedStack<> ?
> > > > 
> > > > HeapLinkedStack is still used in RuleSet.cpp and HeapTest.cpp.
> > > 
> > > Yes, so it makes some sense to keep a symmetric set (Heap and not) around.
> > 
> > Even if LinkedStack is no longer tested (beyond compilation testing)?
> > 
> > It looks like HeapLinkedStack is currently only used as intermediate storage
> > before compacting them into HeapTerminatedArray's.  I could investigate
> cutting
> > out the middleman, if you think it's worthwhile (and can help me figure out
> how
> > to mem + perf test such a change).  If not, I can at least add some
> > documentation to these classes.
> 
> +1 to removing HeapLinkedStack. Reducing # of data structures in the code base
> sounds nice.

RuleSets switched to this stack abstraction to reduce allocation footprint
(i.e., used instead of Vector), iirc.

If HeapLinkedStack<> could have a comment next to it to say that it was based on
wtf/LinkedStack.h, that would be preferable -- giving enough of a hint to avoid
re-invention if someone should need the non-heap alternative at some point.

[Mostyn Bramley-Moore, 2017-03-21 09:53:04]

Description was changed from

==========
LinkedStack.h is no longer used, remove it

BUG=none
==========

to

==========
LinkedStack.h is no longer used, remove it

BUG=none
NO_DEPENDENCY_CHECKS=true
==========

[Mostyn Bramley-Moore, 2017-03-21 09:57:17]

On 2017/03/21 06:36:53, sof wrote:
> RuleSets switched to this stack abstraction to reduce allocation footprint
> (i.e., used instead of Vector), iirc.
> 
> If HeapLinkedStack<> could have a comment next to it to say that it was based
on
> wtf/LinkedStack.h, that would be preferable -- giving enough of a hint to
avoid
> re-invention if someone should need the non-heap alternative at some point.

Added such a comment in HeapLinkedStack.h instead- how does this look?

[sof, 2017-03-21 09:58:12]

lgtm

[Mostyn Bramley-Moore, 2017-03-21 10:00:44]

The CQ bit was checked by mostynb@opera.com

[Mostyn Bramley-Moore, 2017-03-21 10:00:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
dglazkov@chromium.org
Link to the patchset: https://codereview.chromium.org/2761853003/#ps20001
(title: "add comment in HeapLinkedStack.h")

[commit-bot: I haz the power, 2017-03-21 10:00:57]

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-21 10:08:32]

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

[commit-bot: I haz the power, 2017-03-21 10:08:33]

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...)

[Mostyn Bramley-Moore, 2017-03-21 10:12:32]

The CQ bit was checked by mostynb@opera.com

[Mostyn Bramley-Moore, 2017-03-21 10:12:32]

The patchset sent to the CQ was uploaded after l-g-t-m from sigbjornf@opera.com,
dglazkov@chromium.org
Link to the patchset: https://codereview.chromium.org/2761853003/#ps40001
(title: "rebase on master, avoid spurious patch dep?")

[commit-bot: I haz the power, 2017-03-21 10:12:44]

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

[haraken, 2017-03-21 10:33:09]

LGTM

[commit-bot: I haz the power, 2017-03-21 11:58:46]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490091152506860,
"parent_rev": "558e1295655666d3e80365af85de9b27b912eb6d", "commit_rev":
"951df7be3b2b83f682351f98300354f7a2807be9"}

[commit-bot: I haz the power, 2017-03-21 11:59:32]

Description was changed from

==========
LinkedStack.h is no longer used, remove it

BUG=none
NO_DEPENDENCY_CHECKS=true
==========

to

==========
LinkedStack.h is no longer used, remove it

BUG=none
NO_DEPENDENCY_CHECKS=true

Review-Url: https://codereview.chromium.org/2761853003
Cr-Commit-Position: refs/heads/master@{#458376}
Committed:
https://chromium.googlesource.com/chromium/src/+/951df7be3b2b83f682351f983003...
==========

[commit-bot: I haz the power, 2017-03-21 11:59:33]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/951df7be3b2b83f682351f983003...