Create same number of mem descs w/ and w/o --devel
I found the number of AST nodes created with and without --devel
to differ. Here is what I believe was the cause.
callChplHereAlloc() invoked newMemDesc() passing it the name of
the type being allocated. newMemDesc() created a new memory descriptor
for each new name that it got. That involved new_IntSymbol() and so
a new VarSymbol + new DefExpr.
When compiling with --no-devel, type names for domains/arrays are
made nicer, e.g.:
Cf. with --devel they are more verbose, e.g.:
BTW they come from the following locations, respectively:
when compiling $CHPL_HOME/test/users/engin/sparse_bulk_dist/sparseBulk.chpl -ssparseLayoutType=CSR
In a couple of cases, such as this, different types have the same
(nicer) name with --no-devel whereas different names with --devel.
So the latter caused extra new_IntSymbol() and so extra two nodes
compared to the former.
WITH THIS CHANGE
newMemDesc now accepts and hashes on the Type* to be allocated,
so distinct types always get distinct memory descriptors
regardless of --devel or not.
As a result, in the above compilation the number of AST nodes
matches between --devel and --no-devel until insertLineNumbers().
insertLineNumbers and codegen still add more nodes with --no-devel
than with --devel. For the above compilation I got ~13k extra in
insertLineNumbers and another ~1.1k in codegen.
(I did not investigate these.)
Interestingly, all calls to newMemDesc resulting in new descriptors
come from resolution - except for one. It happens during scopeResolve
calling callChplHereAlloc(fn->_this->type) (was callChplHereAlloc(fn->_this))
from within build_constructor(AggregateType* ct)
invoked on the generic _ref type.
The corresponding descriptor was associated with "this", now with "_ref".
I did not investigate what becomes of this allocation
when the constructor for _ref is instantiated, if it ever is.
(I did not see any "this" descriptors from nightly memleaks testing.)
This seems to be the first call to newMemDesc, so the memory descriptor is 0.
As suggested by Michael, I made newMemDesc() create separate
descriptor numbers based on the type, rather than its name.
* This required a deviation from name-centric memory descriptors.
In a few cases newMemDesc() is invoked on char* literals e.g.
"global heap-converted data", "bundled args", etc.
Prior to this change, there was a 1:1 between the name and
the descriptor number. Now the same type name can have several
descriptors if multiple Types have the same name. See the above
newMemDesc() still has an implementation that accepts strings.
It uses a separate hash table and issues distinct descriptors
from those on Types.
Michael and I discussed merging the two hash tables into one,
however did not find an easy solution, and it did not seem
worth it to me to try to merge them.
* To implement that, I created a separate newMemDesc() and memDescsMap
that operate on Types. Note that memDescsNodeMap hashes on Type ID,
not the Type* pointer. This is to guard against the case where a Type
is removed from tree and its pointer is reused. Even though such
a case is unlikely.
Michael has reservations about it, perhaps based on the rest
of the compiler using node pointer rather than node ID for hashing
in cases like this. I have been bitten once by pointer-based hashing
when the node used as the key got deallocated then another node
got allocated at the same address. The Map gave me an incorrect
node as a result. Given that I am unaware of the policy that Type
nodes are never deleted (even though in reality that may well be the
case), I am being extra cautious and using IDs for hashing.
However, we can revert to hashing on pointers if desired.
MORE MEMORY DESCRIPTORS NOW - esp. for _ic_ types
The names associated with memory descriptors in the compiled program's
output - those are still always char* - are visible e.g. when running
This is most prominent for iterator classes, they are named _ic_....
Now we create separate descriptors for ICs corresponding to serial,
leader, follower, and standalone iterators (when applicable),
whereas before they were collapsed into one. I left it this way
encouraged by Brad. We can still collapse them by dispatching calls
on IC types to the string-based implementation of newMemDesc.
Of we can be fancy and extend the names for memory descriptors
of IC types with "(leader)", "(follower)", etc.
So now we may be observing multiple lines with the same descriptor name
instead of a single line with combined counts.
I did not come up with an example illustrating that. The test
discussed above leaks those arrays, however it does not illustrate.
We might see something in the nightly memleak testing where we do leak
_ic_ things occasionally.
* I modified callChplHereAlloc() to always accept Type*
instead of Symbol*, thus avoiding Type*->Symbol*->Type* conversion.
The only place where this was non-trivial was replacement:
callChplHereAlloc(fn->_this) --> callChplHereAlloc(fn->_this->type)
(See above for a remark on this.)
* Removed a couple of unnecessary lines from callChplHereAlloc().
* Removed an unnecessary SET_LINENO() call from scopeResolve().
* I considered removing the optional 'md' formal in callChplHereAlloc()
because it is currently unused. I chose to leave it there just in case
and for symmetry with insertChplHereAlloc.