Project

General

Profile

Misc #14762

[PATCH] gc.c: use ccan/list

Added by normalperson (Eric Wong) over 1 year ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
-
[ruby-core:87049]

Description

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


Files

0001-gc.c-use-ccan-list.patch (8.72 KB) 0001-gc.c-use-ccan-list.patch normalperson (Eric Wong), 05/16/2018 01:09 AM

Associated revisions

Revision a3e73d13
Added by normal over 1 year ago

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]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@63447 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 63447
Added by normalperson (Eric Wong) over 1 year ago

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]

Revision 63447
Added by normal over 1 year ago

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]

History

Updated by ko1 (Koichi Sasada) over 1 year ago

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

Updated by normalperson (Eric Wong) over 1 year ago

ko1@atdot.net 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)

#3

Updated by normalperson (Eric Wong) over 1 year ago

  • 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]

Also available in: Atom PDF