Re: [Maria-developers] 21eb8969ce9: Improved storage size for Item, Field and some other classes
Hi, Monty! Looks ok, but again it doesn't seem you've squashed intermediate commits as you said you did. Bit fields and non-existent commits in columnstore - it's is clearly an intermediate work-in-progress state, all fixed in your later commits. On Sep 08, Michael Widenius wrote:
revision-id: 21eb8969ce9 (mariadb-10.5.2-270-g21eb8969ce9) parent(s): c3ecf0d6243 author: Michael Widenius
committer: Michael Widenius timestamp: 2020-09-02 20:58:34 +0300 message: Improved storage size for Item, Field and some other classes
- Changed order of class fields to remove dead alignment space. - Changed bool fields in Item to bit fields. - Used packed enum's for some fields in common classes - Removed not used Item::rsize. - Changed some class variables from uint/int to smaller type int's. - Ensured that field_index is uint16 in all classes and functions. Fixed also that we proparly compare with NO_CACHED_FIELD_INDEX when checking if variable is not set. - Removed checking of highest bit of unireg_check (has not been used in a long time) - Fixed wrong arguments to make_cond_for_table() for join_tab_idx_arg from false to 0.
One of the result was reducing the size if class Item with ~24 bytes
...
@@ -929,18 +904,48 @@ class Item: public Value_source, + + /* + str_values's main purpose is to be used to cache the value in + save_in_field + */ + String str_value; + + LEX_CSTRING name; /* Name of item */ + /* Original item name (if it was renamed)*/ + const char *orig_name; + + uint32 /* All common bool variables for Item stored here */ + maybe_null:1, /* If item may be null */ + in_rollup:1, /* If used in GROUP BY list of a query with ROLLUP */ + with_param:1, /* True if Item contains an SP parameter */ + with_window_func:1, /* True if item contains a window func */ + with_field:1, /* True if any item except Item_sum contains a field. + Set during parsing. */ + fixed:1; /* If item was fixed with fix_fields */ + + int16 marker; + diff --git a/storage/columnstore/columnstore b/storage/columnstore/columnstore index b6b02ed516f..f606e76fb77 160000 --- a/storage/columnstore/columnstore +++ b/storage/columnstore/columnstore @@ -1 +1 @@ -Subproject commit b6b02ed516f92055127d416370799d91a82754ea +Subproject commit f606e76fb779e40f3376693fff9969e4f2b7669a
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi!
On Fri, Sep 11, 2020 at 1:10 PM Sergei Golubchik
Hi, Monty!
Looks ok, but again it doesn't seem you've squashed intermediate commits as you said you did.
I have squashed everything that makes sense. I did leave commits that shows me trying with bit fields and then moving to flags as these can be useful for anyone following the development process. It also allows one to checkout the bitfield code and run tests with it.
Bit fields and non-existent commits in columnstore - it's is clearly an intermediate work-in-progress state, all fixed in your later commits.
For columnstore I added a commit on top to allow columnstore to compile, yes. That was the best way to get this work done. As soon I have pushed, I hope the columnstore team will take the patches and add it to standard columnstore... <cut> Regards, Monty
Hi, Michael! On Sep 25, Michael Widenius wrote:
Hi!
On Fri, Sep 11, 2020 at 1:10 PM Sergei Golubchik
wrote: Hi, Monty!
Looks ok, but again it doesn't seem you've squashed intermediate commits as you said you did.
I have squashed everything that makes sense. I did leave commits that shows me trying with bit fields and then moving to flags as these can be useful for anyone following the development process. It also allows one to checkout the bitfield code and run tests with it.
I'd rather see commits to "run tests with it" in a separate branch, like, 10.6-bitfields. Not in the main branch, they only muddle the history and complicate change analysys. If you'd like I can create a separate branch for bitfields and squash commits in the main branch into something that is easier to digest.
Bit fields and non-existent commits in columnstore - it's is clearly an intermediate work-in-progress state, all fixed in your later commits.
For columnstore I added a commit on top to allow columnstore to compile, yes. That was the best way to get this work done. As soon I have pushed, I hope the columnstore team will take the patches and add it to standard columnstore...
Right. But non-existent commits mean you cannot check out the bitfield code and run tests with it. It won't compile because columnstore will fail to check out and cmake will stop before any compilation will even be able to start. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi! <cut>
Right. But non-existent commits mean you cannot check out the bitfield code and run tests with it. It won't compile because columnstore will fail to check out and cmake will stop before any compilation will even be able to start.
I don't really care if one for 3-5 commits in a stack of 80 cannot compile with Columnstore. What I care of is to record in the source the iterations of bit fields. Regards, Monty
participants (2)
-
Michael Widenius
-
Sergei Golubchik