Reduce pack-objects memory footprint?

PATCH Superseded Duy Nguyen 8 participants 273 messages Feb 28, 2018 – Apr 6, 2018 Documentation Page 1 of 3

Share & tools

Duy Nguyen

author

2018-02-28 09:27 UTC #0 lore raw

linux-2.6.git current has 6483999 objects. "git gc" on my poor laptop consumes 1.7G out of 4G RAM, pushing lots of data to swap and making all apps nearly unusuable (granted the problem is partly Linux I/O scheduler too). So I wonder if we can reduce pack-objects memory footprint a bit.

This demonstration patch (probably breaks some tests) would reduce the size of struct object_entry from from 136 down to 112 bytes on x86-64. There are 6483999 of these objects, so the saving is 17% or 148 MB.

If we go further, notice that nr_objects is uint32_t, we could convert the three pointers

	struct object_entry *delta;
	struct object_entry *delta_child;
	struct object_entry *delta_sibling;

	uint32_t delta;
	uint32_t delta_child;
	uint32_t delta_sibling;

which saves 12 bytes (or another 74 MB). 222 MB total is plenty of space to keep some file cache from being evicted.

Is it worth doing this? The struct packing makes it harder to read (and more fragile too). I added some more artifical limit like max depth of 2^11. But I think 4096+ depth is getting unreasonable.

-- 8< --

builtin/pack-objects.c +11 -4

index 5c674b2843..6a9804daec 100644
@@ -877,8 +877,11 @@ static void write_pack_file(void)
 			strbuf_addf(&tmpname, "%s-", base_name);
 
 			if (write_bitmap_index) {
+				ALLOC_ARRAY(to_pack.in_pack_pos, to_pack.nr_objects);
 				bitmap_writer_set_checksum(oid.hash);
-				bitmap_writer_build_type_index(written_list, nr_written);
+				bitmap_writer_build_type_index(written_list,
+							       nr_written,
+							       &to_pack);
 			}
 
 			finish_tmp_packfile(&tmpname, pack_tmp_name,

@@ -1407,6 +1410,7 @@ static void check_object(struct object_entry *entry)
 		unsigned long avail;
 		off_t ofs;
 		unsigned char *buf, c;
+		enum object_type type;
 
 		buf = use_pack(p, &w_curs, entry->in_pack_offset, &avail);

@@ -1415,8 +1419,9 @@ static void check_object(struct object_entry *entry)
 		 * since non-delta representations could still be reused.
 		 */
 		used = unpack_object_header_buffer(buf, avail,
-						   &entry->in_pack_type,
+						   &type,
 						   &entry->size);
+		entry->in_pack_type = type;
 		if (used == 0)
 			goto give_up;

@@ -1559,6 +1564,7 @@ static void drop_reused_delta(struct object_entry *entry)
 {
 	struct object_entry **p = &entry->delta->delta_child;
 	struct object_info oi = OBJECT_INFO_INIT;
+	enum object_type type;
 
 	while (*p) {
 		if (*p == entry)

@@ -1570,7 +1576,7 @@ static void drop_reused_delta(struct object_entry *entry)
 	entry->depth = 0;
 
 	oi.sizep = &entry->size;
-	oi.typep = &entry->type;
+	oi.typep = &type;
 	if (packed_object_info(entry->in_pack, entry->in_pack_offset, &oi) < 0) {
 		/*
 		 * We failed to get the info from this pack for some reason;

@@ -1580,7 +1586,8 @@ static void drop_reused_delta(struct object_entry *entry)
 		 */
 		entry->type = sha1_object_info(entry->idx.oid.hash,
 					       &entry->size);
-	}
+	} else
+		entry->type = type;
 }
 
 /*

pack-bitmap-write.c +4 -3

index e01f992884..5c9957a095 100644
@@ -49,7 +49,8 @@ void bitmap_writer_show_progress(int show)
  * Build the initial type index for the packfile
  */
 void bitmap_writer_build_type_index(struct pack_idx_entry **index,
-				    uint32_t index_nr)
+				    uint32_t index_nr,
+				    struct packing_data *to_pack)
 {
 	uint32_t i;

@@ -62,7 +63,7 @@ void bitmap_writer_build_type_index(struct pack_idx_entry **index,
 		struct object_entry *entry = (struct object_entry *)index[i];
 		enum object_type real_type;
 
-		entry->in_pack_pos = i;
+		to_pack->in_pack_pos[entry - to_pack->objects] = i;
 
 		switch (entry->type) {
 		case OBJ_COMMIT:

@@ -147,7 +148,7 @@ static uint32_t find_object_pos(const unsigned char *sha1)
 			"(object %s is missing)", sha1_to_hex(sha1));
 	}
 
-	return entry->in_pack_pos;
+	return writer.to_pack->in_pack_pos[entry - writer.to_pack->objects];
 }
 
 static void show_object(struct object *object, const char *name, void *data)

pack-bitmap.c +1 -1

index 9270983e5f..595914fa43 100644
@@ -1032,7 +1032,7 @@ int rebuild_existing_bitmaps(struct packing_data *mapping,
 		oe = packlist_find(mapping, sha1, NULL);
 
 		if (oe)
-			reposition[i] = oe->in_pack_pos + 1;
+			reposition[i] = mapping->in_pack_pos[oe - mapping->objects] + 1;
 	}
 
 	rebuild = bitmap_new();

pack-bitmap.h +3 -1

index 3742a00e14..2558f7662a 100644
@@ -44,7 +44,9 @@ int rebuild_existing_bitmaps(struct packing_data *mapping, khash_sha1 *reused_bi
 
 void bitmap_writer_show_progress(int show);
 void bitmap_writer_set_checksum(unsigned char *sha1);
-void bitmap_writer_build_type_index(struct pack_idx_entry **index, uint32_t index_nr);
+void bitmap_writer_build_type_index(struct pack_idx_entry **index,
+				    uint32_t index_nr,
+				    struct packing_data *);
 void bitmap_writer_reuse_bitmaps(struct packing_data *to_pack);
 void bitmap_writer_select_commits(struct commit **indexed_commits,
 		unsigned int indexed_commits_nr, int max_bitmaps);

pack-objects.h +18 ~~-16~~

index 03f1191659..caecad23b6 100644
@@ -1,6 +1,18 @@
 #ifndef PACK_OBJECTS_H
 #define PACK_OBJECTS_H
 
+/*
+ * State flags for depth-first search used for analyzing delta cycles.
+ *
+ * The depth is measured in delta-links to the base (so if A is a delta
+ * against B, then A has a depth of 1, and B a depth of 0).
+ */
+enum dfs_state {
+	DFS_NONE = 0,
+	DFS_ACTIVE,
+	DFS_DONE
+};
+
 struct object_entry {
 	struct pack_idx_entry idx;
 	unsigned long size;	/* uncompressed size */

@@ -14,11 +26,10 @@ struct object_entry {
 	void *delta_data;	/* cached delta (uncompressed) */
 	unsigned long delta_size;	/* delta data size (uncompressed) */
 	unsigned long z_delta_size;	/* delta data size (compressed) */
-	enum object_type type;
-	enum object_type in_pack_type;	/* could be delta */
 	uint32_t hash;			/* name hint hash */
-	unsigned int in_pack_pos;
 	unsigned char in_pack_header_size;
+	unsigned type:3;	 /* enum object_type */
+	unsigned in_pack_type:3; /* enum object_type - could be delta */
 	unsigned preferred_base:1; /*
 				    * we do not pack this, but is available
 				    * to be used as the base object to delta

@@ -27,19 +38,8 @@ struct object_entry {
 	unsigned no_try_delta:1;
 	unsigned tagged:1; /* near the very tip of refs */
 	unsigned filled:1; /* assigned write-order */
-
-	/*
-	 * State flags for depth-first search used for analyzing delta cycles.
-	 *
-	 * The depth is measured in delta-links to the base (so if A is a delta
-	 * against B, then A has a depth of 1, and B a depth of 0).
-	 */
-	enum {
-		DFS_NONE = 0,
-		DFS_ACTIVE,
-		DFS_DONE
-	} dfs_state;
-	int depth;
+	unsigned dfs_state:3;	/* enum dfs_state */
+	unsigned depth:11;
 };
 
 struct packing_data {

@@ -48,6 +48,8 @@ struct packing_data {
 
 	int32_t *index;
 	uint32_t index_size;
+
+	int *in_pack_pos;
 };
 
 struct object_entry *packlist_alloc(struct packing_data *pdata,

-- 8< --

Jeff King

2018-02-28 10:18 UTC #1 lore raw

Quoted 5 lines · Duy Nguyen

linux-2.6.git current has 6483999 objects. "git gc" on my poor laptop consumes 1.7G out of 4G RAM, pushing lots of data to swap and making all apps nearly unusuable (granted the problem is partly Linux I/O scheduler too). So I wonder if we can reduce pack-objects memory footprint a bit.

Yeah, the per object memory footprint is not great. Around 100 million objects it becomes pretty ridiculous. I started to dig into it a year or three ago when I saw such a case, but it turned out to be something that we could prune. The torvalds/linux fork network has ~23 million objects, so it's probably 7-8 GB of book-keeping. Which is gross, but 64GB in a server isn't uncommon these days.

I think laptops repacking the kernel are probably one of the worst cases (leaving aside the awful Windows repository, but my impression is that they simply can't do a full repack at all there).

Quoted 4 lines

This demonstration patch (probably breaks some tests) would reduce the size of struct object_entry from from 136 down to 112 bytes on x86-64. There are 6483999 of these objects, so the saving is 17% or 148 MB.

136 x 6.5M objects is only about 800MB. I suspect a big chunk of the rest is going to the object structs we create when doing the internal rev-list traversal. And those duplicate the 20-byte sha1s at the very least.

I don't know if it would be a good idea to free them after the traversal, though. We do use them again later in the bitmap case. On the other hand, we could probably do so for the non-bitmap case. And even for the bitmap case, the primary value in keeping them around is that the parent pointers will already be cached. So it might make sense to free the blobs and trees (though it might be tricky; the commits have pointers to the trees).

It also doesn't help with peak memory usage, because you'll have the full to_pack list and all of the "struct object" in memory together at one point.

Another option would be to somehow replace the pack_idx_entry with a reference to a "struct object". That would let us at least avoid storing the 20-byte oid twice.

Quoted 7 lines

If we go further, notice that nr_objects is uint32_t, we could convert the three pointers
	struct object_entry *delta;
	struct object_entry *delta_child;
	struct object_entry *delta_sibling;

Quoted 8 lines

to
	uint32_t delta;
	uint32_t delta_child;
	uint32_t delta_sibling;
which saves 12 bytes (or another 74 MB). 222 MB total is plenty of space to keep some file cache from being evicted.

Yeah, that seems like low-hanging fruit. I'd also note that we don't actually use all of the fields during the whole process. I think some of those delta fields are only used for a short time. So we might be able to reduce peak memory if there are some mutually exclusive bits of each entry (and even if there's some overlap, we'd reduce the length of time we'd need to be at peak).

Quoted 3 lines

Is it worth doing this? The struct packing makes it harder to read (and more fragile too). I added some more artifical limit like max depth of 2^11. But I think 4096+ depth is getting unreasonable.

I'm OK with a limit like 4096, as long as we notice when we hit the limit and behave reasonably. I think the algorithm in break_delta_chains() may temporarily store up to uint32_t depth. But I think we won't ever write anything into cur->depth larger than the max-depth limit. So it would probably be sufficient to just check that the --depth argument is reasonably sized and complain otherwise.

I do agree this makes things a bit harder to read, but I think the benefits are pretty measurable. And may make a real usability difference on a large repository.

-Peff

Duy Nguyen

author

2018-02-28 10:59 UTC #2 lore raw

Quoted 8 lines · Jeff King

Quoted 5 lines · Duy Nguyen
linux-2.6.git current has 6483999 objects. "git gc" on my poor laptop consumes 1.7G out of 4G RAM, pushing lots of data to swap and making all apps nearly unusuable (granted the problem is partly Linux I/O scheduler too). So I wonder if we can reduce pack-objects memory footprint a bit.

Quoted 4 lines

Yeah, the per object memory footprint is not great. Around 100 million objects it becomes pretty ridiculous. I started to dig into it a year or three ago when I saw such a case, but it turned out to be something that we could prune.

We could? What could we prune?

Quoted 3 lines

The torvalds/linux fork network has ~23 million objects,
so it's probably 7-8 GB of book-keeping. Which is gross, but 64GB in a server isn't uncommon these days.

I wonder if we could just do book keeping for some but not all objects because all objects simply do not scale. Say we have a big pack of many GBs, could we keep the 80% of its bottom untouched, register the top 20% (mostly non-blobs, and some more blobs as delta base) for repack? We copy the bottom part to the new pack byte-by-byte, then pack-objects rebuilds the top part with objects from other sources.

That would of course be less optimal because we can't make delta against those objects at the bottom. And this makes the assumption that these packs are generated using the same heuristics that we use.

Quoted 3 lines

I think laptops repacking the kernel are probably one of the worst cases (leaving aside the awful Windows repository, but my impression is that they simply can't do a full repack at all there).

Yeah. My fear is "git gc --auto" kicking in. Even on a laptop we should support working on a repo as big as linux-2.6 (or as smalls as one from facebook/microsoft perspective). I probably will add a mode that keeps the largest pack alone and just pack the rest in a second pack in "git gc --auto". That might help (I haven't really checked the details yet, but object_entry book keeping should go down at least)

Quoted 9 lines

Quoted 4 lines
This demonstration patch (probably breaks some tests) would reduce the size of struct object_entry from from 136 down to 112 bytes on x86-64. There are 6483999 of these objects, so the saving is 17% or 148 MB.
136 x 6.5M objects is only about 800MB. I suspect a big chunk of the rest is going to the object structs we create when doing the internal rev-list traversal. And those duplicate the 20-byte sha1s at the very least.

They are 32 bytes per entry, so it should take less than object_entry. I briefly wondered if we should fall back to external rev-list too, just to free that memory.

So about 200 MB for those objects (or maybe more for commits). Add 256 MB delta cache on top, it's still a bit far from 1.7G. There's something I'm still missing.

Quoted 3 lines

Another option would be to somehow replace the pack_idx_entry with a reference to a "struct object". That would let us at least avoid storing the 20-byte oid twice.

20 bytes saving? /me drools.

Pity we can't do the same for 'struct object'. Most of the time we have a giant .idx file with most hashes. We could look up in both
places: the hash table in object.c, and the idx file, to find an object. Then those objects that are associated with .idx file will not need "oid" field (needed to as key for the hash table). But I see no way to make that change.

Quoted 16 lines

Quoted 7 lines
If we go further, notice that nr_objects is uint32_t, we could convert the three pointers
      struct object_entry *delta;
      struct object_entry *delta_child;
      struct object_entry *delta_sibling;
Quoted 8 lines
to
      uint32_t delta;
      uint32_t delta_child;
      uint32_t delta_sibling;
which saves 12 bytes (or another 74 MB). 222 MB total is plenty of space to keep some file cache from being evicted.

Quoted 6 lines

Yeah, that seems like low-hanging fruit. I'd also note that we don't actually use all of the fields during the whole process. I think some of those delta fields are only used for a short time. So we might be able to reduce peak memory if there are some mutually exclusive bits of each entry (and even if there's some overlap, we'd reduce the length of time we'd need to be at peak).

Yeah it looks that way to me too. Now that we have packing_data, splitting object_entry[] to multiple arrays (that is only needed at some point) may be very nice.

Quoted 11 lines

Quoted 3 lines
Is it worth doing this? The struct packing makes it harder to read (and more fragile too). I added some more artifical limit like max depth of 2^11. But I think 4096+ depth is getting unreasonable.
I'm OK with a limit like 4096, as long as we notice when we hit the limit and behave reasonably. I think the algorithm in break_delta_chains() may temporarily store up to uint32_t depth. But I think we won't ever write anything into cur->depth larger than the max-depth limit. So it would probably be sufficient to just check that the --depth argument is reasonably sized and complain otherwise.

Quoted 3 lines

I do agree this makes things a bit harder to read, but I think the benefits are pretty measurable. And may make a real usability difference on a large repository.

Thanks. I'll go make real patches then (with lots more checks to make sure we don't overflow in shortened fields like depth or type).

Signature · 1 line

Duy

Jeff King

2018-02-28 11:11 UTC #3 lore raw

Quoted 6 lines · Duy Nguyen

Quoted 4 lines
Yeah, the per object memory footprint is not great. Around 100 million objects it becomes pretty ridiculous. I started to dig into it a year or three ago when I saw such a case, but it turned out to be something that we could prune.
We could? What could we prune?

Sorry, I just meant that my 100 million-object case turned out not to need all those objects, and I was able to prune it down. No code fixes came out of it. ;)

Quoted 10 lines

Quoted 3 lines
The torvalds/linux fork network has ~23 million objects,
so it's probably 7-8 GB of book-keeping. Which is gross, but 64GB in a server isn't uncommon these days.
I wonder if we could just do book keeping for some but not all objects because all objects simply do not scale. Say we have a big pack of many GBs, could we keep the 80% of its bottom untouched, register the top 20% (mostly non-blobs, and some more blobs as delta base) for repack? We copy the bottom part to the new pack byte-by-byte, then pack-objects rebuilds the top part with objects from other sources.

Yes, though I think it would take a fair bit of surgery to do internally. And some features (like bitmap generation) just wouldn't work at all.

I suspect you could simulate it, though, by just packing your subset with pack-objects (feeding it directly without using "--revs") and then catting the resulting packfiles together with a fixed-up header.

At one point I played with a "fast pack" that would just cat packfiles together. My goal was to make cases with 10,000 packs workable by creating one lousy pack, and then repacking that lousy pack with a "real" repack. In the end I abandoned it in favor of fixing the performance problems from trying to make a real pack of 10,000 packs. :)

But I might be able to dig it up if you want to experiment in that direction.

Quoted 7 lines

They are 32 bytes per entry, so it should take less than object_entry. I briefly wondered if we should fall back to external rev-list too, just to free that memory.
So about 200 MB for those objects (or maybe more for commits). Add 256 MB delta cache on top, it's still a bit far from 1.7G. There's something I'm still missing.

Are you looking at RSS or heap? Keep in mind that you're mmap-ing what's probably a 1GB packfile on disk. If you're under memory pressure that won't all stay resident, but some of it will be counted in RSS.

Quoted 6 lines

Pity we can't do the same for 'struct object'. Most of the time we have a giant .idx file with most hashes. We could look up in both
places: the hash table in object.c, and the idx file, to find an object. Then those objects that are associated with .idx file will not need "oid" field (needed to as key for the hash table). But I see no way to make that change.

Yeah, that would be pretty invasive, I think. I also wonder if it would perform worse due to cache effects.

-Peff

Duy Nguyen

author

2018-02-28 11:25 UTC #4 lore raw

Quoted 19 lines · Jeff King

Quoted 10 lines
Quoted 3 lines
The torvalds/linux fork network has ~23 million objects,
so it's probably 7-8 GB of book-keeping. Which is gross, but 64GB in a server isn't uncommon these days.
I wonder if we could just do book keeping for some but not all objects because all objects simply do not scale. Say we have a big pack of many GBs, could we keep the 80% of its bottom untouched, register the top 20% (mostly non-blobs, and some more blobs as delta base) for repack? We copy the bottom part to the new pack byte-by-byte, then pack-objects rebuilds the top part with objects from other sources.
Yes, though I think it would take a fair bit of surgery to do internally. And some features (like bitmap generation) just wouldn't work at all.
I suspect you could simulate it, though, by just packing your subset with pack-objects (feeding it directly without using "--revs") and then catting the resulting packfiles together with a fixed-up header.

Quoted 8 lines

At one point I played with a "fast pack" that would just cat packfiles together. My goal was to make cases with 10,000 packs workable by creating one lousy pack, and then repacking that lousy pack with a "real" repack. In the end I abandoned it in favor of fixing the performance problems from trying to make a real pack of 10,000 packs. :)
But I might be able to dig it up if you want to experiment in that direction.

Naah it's ok. I'll go similar direction, but I'd repack those pack files too except the big one. Let's see how it turns out.

Quoted 11 lines

Quoted 7 lines
They are 32 bytes per entry, so it should take less than object_entry. I briefly wondered if we should fall back to external rev-list too, just to free that memory.
So about 200 MB for those objects (or maybe more for commits). Add 256 MB delta cache on top, it's still a bit far from 1.7G. There's something I'm still missing.
Are you looking at RSS or heap? Keep in mind that you're mmap-ing what's probably a 1GB packfile on disk. If you're under memory pressure that won't all stay resident, but some of it will be counted in RSS.

Interesting. It was RSS.

Quoted 9 lines

Quoted 6 lines
Pity we can't do the same for 'struct object'. Most of the time we have a giant .idx file with most hashes. We could look up in both
places: the hash table in object.c, and the idx file, to find an object. Then those objects that are associated with .idx file will not need "oid" field (needed to as key for the hash table). But I see no way to make that change.
Yeah, that would be pretty invasive, I think. I also wonder if it would perform worse due to cache effects.

It should be better because of cache effects, I think. I mean, hash map is the least cache friendly lookup. Moving most objects out of the hash table shrinks it, which is even nicer to cache. But we also lose O(1) when we do binary search on .idx file (after failing to find the same object in the hash table)

Signature · 1 line

Duy

Eric Wong

2018-02-28 18:22 UTC #5 lore raw

In reply to Duy Nguyen

Quoted 2 lines · Duy Nguyen

which saves 12 bytes (or another 74 MB). 222 MB total is plenty of space to keep some file cache from being evicted.

Nice! I can definitely benefit from lower memory usage when packing. Fwiw, I use pahole with other projects to help find packing opportunities:

git://git.kernel.org/pub/scm/devel/pahole/pahole.git

pack-objects.h struct object_entry { · line 32

Quoted 11 lines

pack-objects.h +2 -3

@@ -14,11 +26,10 @@ struct object_entry {
 	void *delta_data;	/* cached delta (uncompressed) */
 	unsigned long delta_size;	/* delta data size (uncompressed) */
 	unsigned long z_delta_size;	/* delta data size (compressed) */
-	enum object_type type;
-	enum object_type in_pack_type;	/* could be delta */
 	uint32_t hash;			/* name hint hash */
-	unsigned int in_pack_pos;
 	unsigned char in_pack_header_size;
+	unsigned type:3;	 /* enum object_type */
+	unsigned in_pack_type:3; /* enum object_type - could be delta */

For C99 compilers, enums can be bitfields. I introduced the following macro into Ruby a few weeks ago to remain compatible with non-C99 compilers:

/*
* For declaring bitfields out of non-unsigned int types:
* struct date {
* BITFIELD(enum months) month:4;
* ...
* };
*/
#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L)

# define BITFIELD(type) type
#else
# define BITFIELD(type) unsigned int
#endif

Duy Nguyen

author

2018-03-01 09:00 UTC #6 lore raw

Quoted 9 lines · Eric Wong

Quoted 2 lines · Duy Nguyen
which saves 12 bytes (or another 74 MB). 222 MB total is plenty of space to keep some file cache from being evicted.
Nice! I can definitely benefit from lower memory usage when packing. Fwiw, I use pahole with other projects to help find packing opportunities:
git://git.kernel.org/pub/scm/devel/pahole/pahole.git

Yes it's a wonderful tool.

Quoted 24 lines

pack-objects.h struct object_entry { · line 32
Quoted 11 lines
pack-objects.h +2 -3
@@ -14,11 +26,10 @@ struct object_entry {
 	void *delta_data;	/* cached delta (uncompressed) */
 	unsigned long delta_size;	/* delta data size (uncompressed) */
 	unsigned long z_delta_size;	/* delta data size (compressed) */
-	enum object_type type;
-	enum object_type in_pack_type;	/* could be delta */
 	uint32_t hash;			/* name hint hash */
-	unsigned int in_pack_pos;
 	unsigned char in_pack_header_size;
+	unsigned type:3;	 /* enum object_type */
+	unsigned in_pack_type:3; /* enum object_type - could be delta */
For C99 compilers, enums can be bitfields. I introduced the following macro into Ruby a few weeks ago to remain compatible with non-C99 compilers:
/*
* For declaring bitfields out of non-unsigned int types:
* struct date {
* BITFIELD(enum months) month:4;
* ...
* };
*/
#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L)

Quoted 4 lines

# define BITFIELD(type) type
#else
# define BITFIELD(type) unsigned int
#endif

I tried this and got

In file included from builtin/pack-objects.c:20:0:
./pack-objects.h:49:19: l?i: ?type? is narrower than values of its type [-Werror]

  enum object_type type:TYPE_BITS;
                   ^~~~

The compiler is not wrong. What it does not realize is pack-objects code never uses out-of-range values (OBJ_BAD and OBJ_ANY) but I don't see how I could suppress this warning. So I went back to non-enum bitfields.