On 3/21/19 1:48 PM, Jakub Jelinek wrote:
On Thu, Mar 21, 2019 at 10:25:25AM -0400, Jason Merrill wrote:
Attached (so far without changelog, selftest additions and only tested with
make -j32 -k check-c++-all RUNTESTFLAGS="dg.exp='pr89767.C *lambda* *desig*'"
) is
1) hash_{table,set} <whatever, true> implementation for lazy allocation

I would expect that a single test on an object that's already in cache in
the context of hash table operations would be insignificant, but if you

Some hash table operations are in performance sensitive code and even if
those checks wouldn't take too long, it would grow I-cache.

really want to offer that choice this seems like a clean enough way to
implement it.  Are you sure that we want the default to be immediate
allocation?

There are many that do want immediate allocation, so changing the default
looks way too risky to me.

2) the PR c++/89767 patch with optimization for zero and one entries
3) incremental PR c++/71446 fix to simplify that code using this new
     infrastructure

2 and 3 are explicitly specifying "false" for the Lazy parameter, which is
the default.

Ugh, thanks for spotting that.  Tests passed with that, and I only went
through in a debugger the earlier version with EMPTY_HASH.

We might do the duplicate checking in a wrapper around add_capture since
it's only used by cp_parser_lambda_introducer.  It's fine either way.

Looks like a good idea to me.

Attached are new versions of all the 3 patches.
1) now has ChangeLog and hash-set-selftest.c coverage
2) has the false -> true fixed
3) ditto, but furthermore is moved out of add_capture into the lambda
introducer parsing routine only, tests for [this, this] and [this, *this]
etc. are done using LAMBDA_EXPR_THIS_CAPTURE and the duplicate check for
capture_id is simplified too.

I needed to make the below change to avoid crashing with --enable-gather-detailed-mem-stats, make sense? Incidentally, why not set m_size to 0 when we haven't allocated anything yet?

Jason
commit a56f5f0cb4d4b2c94f181f6a5e029e41baa9b9d4
Author: Jason Merrill <ja...@redhat.com>
Date:   Mon Mar 25 15:07:26 2019 -0400

            * hash-table.h (hash_table): Don't release_instance_overhead if we
            haven't allocated anything.

diff --git a/gcc/hash-table.h b/gcc/hash-table.h
index e5bbe677cec..99c20fb5816 100644
--- a/gcc/hash-table.h
+++ b/gcc/hash-table.h
@@ -652,12 +652,12 @@ hash_table<Descriptor, Lazy, Allocator>::~hash_table ()
        Allocator <value_type> ::data_free (m_entries);
       else
        ggc_free (m_entries);
-    }
 
-  if (m_gather_mem_stats)
-    hash_table_usage ().release_instance_overhead (this,
-                                                  sizeof (value_type)
-                                                  * m_size, true);
+      if (m_gather_mem_stats)
+       hash_table_usage ().release_instance_overhead (this,
+                                                      sizeof (value_type)
+                                                      * m_size, true);
+    }
 }
 
 /* This function returns an array of empty hash table elements.  */

Reply via email to