All of lore.kernel.org
 help / color / mirror / Atom feed
* Crush Map compactness
@ 2016-04-13  8:48 Юрий Соколов
  2016-04-13 12:39 ` Sage Weil
  0 siblings, 1 reply; 2+ messages in thread
From: Юрий Соколов @ 2016-04-13  8:48 UTC (permalink / raw)
  To: ceph-devel

Hello, all.

I suggest couple of improvements to CrushWrapper.

Mostly safe change is replace std::map with btree::btree_map.
It is straightforward change and it looks to be safe.

More complicated change is replacing std::string with custom allocated strings:
- use custom allocator binded to CrushWrapper, - just a bump allocator
with from list of blobs,
- custom strings, used in CrushWrapper's maps are allocated from this
allocator and never freed in one-by-one manner
- custom data-structure for maps to be able to search by std::string
(or tuned btree::btree_map)
- for CrushWrapper's api results, newly allocated std::string are
returned (with C++11 this is already a case, cause reference counting
is forbidden)
- whole allocator with its blobs is freed together with CrushWrapper.
This approach is workable if and only if CrushWrapper is not changed
after construction.
I'm not too confident with Ceph sources to be sure about this property.

Is CrushWrapper "immutable" after it is fully constructed?

If so, I will try to implement this approach.

With regards,
Sokolov Yura aka funny_falcon

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Crush Map compactness
  2016-04-13  8:48 Crush Map compactness Юрий Соколов
@ 2016-04-13 12:39 ` Sage Weil
  0 siblings, 0 replies; 2+ messages in thread
From: Sage Weil @ 2016-04-13 12:39 UTC (permalink / raw)
  To: Юрий
	Соколов
  Cc: ceph-devel

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1720 bytes --]

On Wed, 13 Apr 2016, Юрий Соколов wrote:
> Hello, all.
> 
> I suggest couple of improvements to CrushWrapper.
> 
> Mostly safe change is replace std::map with btree::btree_map.
> It is straightforward change and it looks to be safe.

+1
 
> More complicated change is replacing std::string with custom allocated strings:
> - use custom allocator binded to CrushWrapper, - just a bump allocator
> with from list of blobs,
> - custom strings, used in CrushWrapper's maps are allocated from this
> allocator and never freed in one-by-one manner
> - custom data-structure for maps to be able to search by std::string
> (or tuned btree::btree_map)
> - for CrushWrapper's api results, newly allocated std::string are
> returned (with C++11 this is already a case, cause reference counting
> is forbidden)
> - whole allocator with its blobs is freed together with CrushWrapper.
> This approach is workable if and only if CrushWrapper is not changed
> after construction.
> I'm not too confident with Ceph sources to be sure about this property.

This is a lot more work, but might be worth it.  Most of the time the map 
*is* immutable.  And when it is updated, we do it by decoding a new 
version of the map...

> Is CrushWrapper "immutable" after it is fully constructed?

The problem is that we also have a bunch of mutators (set_*() methods) 
that are called in OSDMonitor.cc when making updates, and they need to be 
supported to.  I would figure out how to support those as well before 
moving forward.

It is probably worth doing some analysis to determine that these 
allocations are really that expensive, too.  Half of the allocations are 
for the map structs themselves, and those aren't going away.

sage

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-04-13 12:39 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-13  8:48 Crush Map compactness Юрий Соколов
2016-04-13 12:39 ` Sage Weil

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.