[MLton] getText{Start,End} considered harmful

[Matthew Fluet]

On Mon, 22 Sep 2008, Wesley W. Terpstra wrote:
> MLton currently uses two functions, GC_getText{Start,End}, to create a giant
> lookup table for profiling. These two functions are among the hardest things
> to supply when porting MLton. It becomes even harder once shared libraries
> are considered, because then one needs to consider which text segment is
> intended. After reading the i386 sys v abi specification, I still have no
> idea how to do this correctly for shared libraries on elf.

My guess is that profiling with shared libraries is a bit much to ask. 
For one, you've worked to ensure that multiple ML shared libraries can be 
used within the same executable, but what would it mean for both of them 
to be profiling libraries -- they would both want to use SIGPROF for their 
own purposes.

Nonetheless, I appreciate the GC_getText{Start,End} difficulties.
There are/were other issues with the approach to time profiling:
   http://mlton.org/pipermail/mlton/2005-November/028283.html
   http://mlton.org/pipermail/mlton-commit/2005-November/000238.html

> It seems to me that not only are these a portability problem, they are
> completely unnecessary. Making a giant lookup table for every address in the
> text segment hardly seems like a worthy goal. I imagine the justification is
> that it has minimal impact on the performance of the program during handling
> of SIGPROF. However, I would contend that a giant lookup table could very
> well be more intrusive than just using a binary search of a compressed
> (repeated sourceSeqIndexes eliminated) sourceMaps.sourceLabels. Even if the
> binary search does have a greater impact, at 1kHZ it will hardly matter.

Running some actual benchmarks (e.g., with the hamlet benchmark, as is 
reported in the above messages) provides real evidence; "I imagine..." and 
"I would contend..." are good hypotheses, but are not experiments.

> So to sum up: could I just kill these off and use a binary search?

I see that such a patch has already been committed, but it would be nice 
to report the actual runtime (and profiling results) impact of such a 
change.