test.check

Make rose trees a deftype

Details

  • Type: Enhancement Enhancement
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code

Description

This turns rose trees into a custom `deftype` that implements `clojure.lang.Indexed` (for backwards compatability - it means you can still use nth on them without any changes, and means we can interchange vectors with them).

It's just a performance patch - I noticed when profiling the test.check suite I have at Yeller that a lot of time was spent creating clojure vectors (it was the highest sample in the suite).

This produced a noticeable difference (at 95% confidence rating as measured across test runs using ministat) on my test suite, but didn't make such a difference on `test.check`'s (no noticeable difference at 95% confidence). I assume that's because of generator complexity.

I've also minorly changed `root` and `children` in rose-tree.clj - they no longer call `nth` twice, just once. This produced again, a minorly noticeable perf difference. I tried as well overriding them to an interface and having `RoseTree` implement it, but apparently the JIT's inlining is smart enough (even in C1) that there's no noticeable difference between that and children/root being implemented with nth.

Activity

Hide
Gary Fredericks added a comment -

I think one difference this patch makes is that (nth rt 2) no longer throws an IndexOutOfBoundsException, which probably doesn't make a difference currently but could make future bugs harder to find. Was this an intentional change?

Otherwise this looks pretty straightforward.

Show
Gary Fredericks added a comment - I think one difference this patch makes is that (nth rt 2) no longer throws an IndexOutOfBoundsException, which probably doesn't make a difference currently but could make future bugs harder to find. Was this an intentional change? Otherwise this looks pretty straightforward.
Hide
Tom Crayford added a comment -

Sent a second patch (it's the one under 15th April in Jira for me).

Show
Tom Crayford added a comment - Sent a second patch (it's the one under 15th April in Jira for me).
Hide
Gary Fredericks added a comment -

Applied on master, thanks!

Show
Gary Fredericks added a comment - Applied on master, thanks!

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: