Misc #14762: [PATCH] gc.c: use ccan/list

• Status: Closed
• Assignee: -

[normalperson (Eric Wong)]

This seems to improve the readability of gc.c a small amount
and it doesn't have any measurable performance impact.

Code reduction is nice, I might commit it soon:

gc.c | 81 +++++++++++++++++++++++++++-----------------------------------------
1 file changed, 32 insertions(+), 49 deletions(-)

[ko1 (Koichi Sasada)]

No strong opinion.

They are memo:

I tried to reorder sweeping list by (a) "full page (all slots are living)" and (b)"can sweep page (there are some free-able slots)" and sweep only (b).
No big improvement though :p

I think using ccan we can move the order, so we can try it later?

If we have any trouble to modify them using more complex strategy, we can revert it.

Trivial comments:

• The field name sweep_pos seems index for me. Maybe sweeping_page or something is fine for me.
• Every time I feel magical for the name node (for CCAN list). Should I endure? (page_node or something is clear, but verbose I agree).

[normalperson (Eric Wong)]

[e-mail] wrote:

No strong opinion.

They are memo:

I tried to reorder sweeping list by (a) "full page (all slots are living)" and (b)"can sweep page (there are some free-able slots)" and sweep only (b).
No big improvement though :p

I think using ccan we can move the order, so we can try it later?

Yes, ccan/list should make reordering and experimentation
simpler.

If we have any trouble to modify them using more complex strategy, we can revert it.

Trivial comments:

• The field name sweep_pos seems index for me. Maybe
  sweeping_page or something is fine for me.

Right, I think of it as an index to the main ->pages list; so I
used '_pos' suffix. I guess sweeping_page along with comment
clarifying it is a pointer inside ->pages is OK.

• Every time I feel magical for the name node (for CCAN
  list). Should I endure? (page_node or something is clear, but
  verbose I agree).

I only wondered about confusion with T_NODE type.
I suppose page_node is OK, but I prefer shorter names
(since I need big fonts)

[normalperson (Eric Wong)]

• Status changed from Open to Closed

Applied in changeset trunk|r63447.

---

gc.c: use ccan/list

This seems to improve the readability of gc.c a small amount
and it doesn't have any measurable performance impact.

[ruby-core:87067] [Misc #14762]