Remove usages of environment-map

[jbouwman]

Environment maps were introduced in order to provide platform-agnostic
dumping of hashtables present in Coalton environment structures.

A review of the three locations where these maps are
present (method-codegen-sym, class-variable-map, superclass-map)
indicates that in all cases, a simple list or alist structure is more
appropriate because of the small number of elements consulted.

In particular, method-codegen-syms are 1-1 with the method-names list,
so no mapping is needed at all.

[Izaakwltn]

My only thought would be to run some sort of benchmark to make sure this is comparable or faster.

Otherwise, looks good!

[YarinHeffes]

Overall, I like the spirit of this PR, but when I compiled coalton and coalton/testwith the following extra lines, there are cases where this function is run as many as 90 times on a single class. On the other hand, this function is, overall, not called that many times. I printed the results below.

(defvar *called-class-variable-map-p-table* (make-hash-table :test 'eq))

(defun class-variable-map (class)
  (format t "Call number ~D on this class." (incf (gethash class *called-class-variable-map-p-table* 0)))
  (loop :with table := (make-hash-table)
        :for var :in (ty-class-class-variables class)
        :for i :from 0
        :do (setf (gethash var table) i)
        :finally (return table)))

Running (asdf:load-system "coalton" :force t)

Called class-variable-map 171 times on TRYINTO
Called class-variable-map 6 times on RANDOMACCESS
Called class-variable-map 2 times on INTOITERATOR
Called class-variable-map 116 times on INTOITERATOR
Called class-variable-map 71 times on FROMITERATOR
Called class-variable-map 2 times on RANDOMACCESS
Called class-variable-map 94 times on ADD
Called class-variable-map 71 times on FIB
Called class-variable-map 2 times on ADD
Called class-variable-map 3 times on FIB
Called class-variable-map 2 times on C
Called class-variable-map 2 times on C
Called class-variable-map 2 times on C
Called class-variable-map 2 times on C
Called class-variable-map 2 times on C
Called class-variable-map 4 times on C
Called class-variable-map 3 times on MOO
Called class-variable-map 2 times on C
Called class-variable-map 2 times on C
Called class-variable-map 2 times on C
Called class-variable-map 84 times on TRYINTO
Called class-variable-map 69 times on INTOITERATOR
Called class-variable-map 46 times on FROMITERATOR
Called class-variable-map 1 times on RANDOMACCESS
Called class-variable-map 2 times on ADD
Called class-variable-map 3 times on FIB
Called class-variable-map 94 times on ADD
Called class-variable-map 71 times on FIB
Called class-variable-map 2 times on ADD
Called class-variable-map 3 times on FIB
Called class-variable-map 171 times on TRYINTO
Called class-variable-map 6 times on RANDOMACCESS
Called class-variable-map 2 times on INTOITERATOR
Called class-variable-map 110 times on INTOITERATOR
Called class-variable-map 68 times on FROMITERATOR
Called class-variable-map 2 times on RANDOMACCESS
Called class-variable-map 94 times on ADD
Called class-variable-map 71 times on FIB
Called class-variable-map 2 times on ADD
Called class-variable-map 3 times on FIB
Called class-variable-map 171 times on TRYINTO
Called class-variable-map 6 times on RANDOMACCESS
Called class-variable-map 2 times on INTOITERATOR
Called class-variable-map 110 times on INTOITERATOR
Called class-variable-map 68 times on FROMITERATOR
Called class-variable-map 2 times on RANDOMACCESS
Called class-variable-map 94 times on ADD
Called class-variable-map 71 times on FIB
Called class-variable-map 2 times on ADD
Called class-variable-map 3 times on FIB
Called class-variable-map 171 times on TRYINTO
Called class-variable-map 6 times on RANDOMACCESS
Called class-variable-map 2 times on INTOITERATOR
Called class-variable-map 54 times on INTOITERATOR
Called class-variable-map 33 times on FROMITERATOR
Called class-variable-map 2 times on RANDOMACCESS

[YarinHeffes]

Other than this, I don't think I have any comments, I like +62 vs -208 LOC 😎

[jbouwman]

And 9 of those are docstrings :)

[jbouwman]

Following up @Izaakwltn's suggestion to check benchmarking, repeated runs of the test suite don't flag obvious performance regressions. That's tempered by the knowledge that there's not good coverage for, e.g., superclasses.

That class-variable-map in particular is almost always trivially small -- never above 7 entries, for standard library and tests -- and possibly ought to be an alist -- and possibly memoized.

Test

(time (dotimes (x 50) (asdf:test-system "coalton")))

Main

Evaluation took:
  238.551 seconds of real time
  180.034131 seconds of total run time (171.736752 user, 8.297379 system)
  [ Real times consist of 12.984 seconds GC time, and 225.567 seconds non-GC time. ]
  [ Run times consist of 12.982 seconds GC time, and 167.053 seconds non-GC time. ]
  75.47% CPU
  7,816,750 forms interpreted
  177,700 lambdas converted
  93,144,529,824 bytes consed

environment-map

Evaluation took:
  203.295 seconds of real time
  179.135658 seconds of total run time (170.922189 user, 8.213469 system)
  [ Real times consist of 13.827 seconds GC time, and 189.468 seconds non-GC time. ]
  [ Run times consist of 13.827 seconds GC time, and 165.309 seconds non-GC time. ]
  88.12% CPU
  7,798,750 forms interpreted
  177,700 lambdas converted
  92,756,680,592 bytes consed

[stylewarning]

I'll take a look.

[stylewarning]

Sorry for the delay on this. I have one very modest request which is to be very explicit on what a data type is.

[stylewarning]

Can you write a comment describing the keys and values here? I don't think "list" is self-evident. Something like

;; an immutable alist of the form ((a1 b1) ...) whose keys are ... and values are ...

clarify if it's an alist of lists or an alist of conses.

I would say this is the sole and biggest negative of this change, we are using an ad hoc data structure.