Re: [PATCH v5 6/8] mem-pool: fill out functionality

2 messages, 2 authors, 2018-07-02 · open the first message on its own page

Re: [PATCH v5 6/8] mem-pool: fill out functionality

From: Junio C Hamano <hidden>
Date: 2018-06-28 19:10:00

Jameson Miller [off-list ref] writes:
+void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src)
+{
+	struct mp_block *p;
+
+	/* Append the blocks from src to dst */
+	if (dst->mp_block && src->mp_block) {
+		/*
+		 * src and dst have blocks, append
+		 * blocks from src to dst.
+		 */
+		p = dst->mp_block;
+		while (p->next_block)
+			p = p->next_block;
+
+		p->next_block = src->mp_block;
Just being curious, but does this interact with the "we carve out
only from the first block" done in step 4/8?  The remaining unused
space in the first block in the src pool would be wasted, which may
not be such a big deal and may not even be worth comparing the
available space in two blocks and picking a larger one.  But we do
want to decide _after_ thinking things through nevertheless.

RE: [PATCH v5 6/8] mem-pool: fill out functionality

From: Jameson Miller <hidden>
Date: 2018-07-02 18:28:25

quoted
+void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src) {
+	struct mp_block *p;
+
+	/* Append the blocks from src to dst */
+	if (dst->mp_block && src->mp_block) {
+		/*
+		 * src and dst have blocks, append
+		 * blocks from src to dst.
+		 */
+		p = dst->mp_block;
+		while (p->next_block)
+			p = p->next_block;
+
+		p->next_block = src->mp_block;
Just being curious, but does this interact with the "we carve out only from the
first block" done in step 4/8?  The remaining unused space in the first block in
the src pool would be wasted, which may not be such a big deal and may not
even be worth comparing the available space in two blocks and picking a larger
one.  But we do want to decide _after_ thinking things through nevertheless.
Good question - and this is something I had thought about.

In the context of current usage, this function is currently not
used frequently. It is only used in split index with
move_cache_to_base_index and remove_split_index. In either case,
the cache_entries should already be loaded into memory, and I
don't expect a big enough difference in the amount of left over
space to make a meaningful difference.

In the general case I could see this being a bigger case.

For now, I thought the logic as is was an appropriate tradeoff. I
don't think it would be complicated to do something smarter here,
but I also didn't see much benefit in current usage. If we prefer
something else here, I can update this logic.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help