Skip to content

Conversation

@JordanMartinez
Copy link
Contributor

These are minor optimizations I realized could be done while I worked on #203

@hdgarrood
Copy link
Contributor

I'm not sure this is safe; aren't you mutating the argument array here?

@hdgarrood
Copy link
Contributor

Oh never mind, you're only calling unsafeThaw on the result of mapWithIndex, which will create a new array so I think we're okay. Have you benchmarked to see how much of a difference there is?

@JordanMartinez
Copy link
Contributor Author

Have you benchmarked to see how much of a difference there is?

Not yet. I'm working on that right now.

@JordanMartinez
Copy link
Contributor Author

Dang... this PR actually slows it down slightly. Branch testing it on current master

Test run:

benchNubBy = do
    log "nubBy"
    log "---------------"
    let shortNats = Array.range 0 100
        longNats = Array.range 0 10000
        mod3Cmp x y = compare (x `mod` 3) (y `mod` 3)

    log $ "nubBy (" <> show (Array.length shortNats) <> ")"
    benchWith 1000 \_ -> Array.nubBy mod3Cmp shortNats

    log $ "nubBy (" <> show (Array.length longNats) <> ")"
    benchWith 100 \_ -> Array.nubBy mod3Cmp longNats
`master` branch - output of compiled `nubBy`
var nubByEq = function (eq) {
    return function (xs) {
        return (function __do() {
            var arr = Data_Array_ST["new"]();
            Control_Monad_ST_Internal.foreach(xs)(function (x) {
                return function __do() {
                    var e = Data_Functor.map(Control_Monad_ST_Internal.functorST)((function () {
                        var $96 = Data_HeytingAlgebra.not(Data_HeytingAlgebra.heytingAlgebraBoolean);
                        var $97 = $foreign.any(function (v) {
                            return eq(v)(x);
                        });
                        return function ($98) {
                            return $96($97($98));
                        };
                    })())(Data_Array_ST.unsafeFreeze(arr))();
                    return Control_Applicative.when(Control_Monad_ST_Internal.applicativeST)(e)(Data_Functor["void"](Control_Monad_ST_Internal.functorST)(Data_Array_ST.push(x)(arr)))();
                };
            })();
            return Data_Array_ST.unsafeFreeze(arr)();
        })();
    };
};
Benchmark

nubBy
---------------
nubBy (101)
mean   = 91.80 μs
stddev = 152.69 μs
min    = 52.02 μs
max    = 3.04 ms
nubBy (10001)
mean   = 9.13 ms
stddev = 3.06 ms
min    = 7.74 ms
max    = 36.80 ms
this PR - output of compiled `nubBy`
var nubByEq = function (eq) {
    return function (xs) {
        return (function __do() {
            var arr = Data_Array_ST["new"]();
            Control_Monad_ST_Internal.foreach(xs)(function (x) {
                return function __do() {
                    var e = Data_Functor.map(Control_Monad_ST_Internal.functorST)((function () {
                        var $96 = Data_HeytingAlgebra.not(Data_HeytingAlgebra.heytingAlgebraBoolean);
                        var $97 = $foreign.any(function (v) {
                            return eq(v)(x);
                        });
                        return function ($98) {
                            return $96($97($98));
                        };
                    })())(Data_Array_ST.unsafeFreeze(arr))();
                    return Control_Applicative.when(Control_Monad_ST_Internal.applicativeST)(e)(Data_Functor["void"](Control_Monad_ST_Internal.functorST)(Data_Array_ST.push(x)(arr)))();
                };
            })();
            return Data_Array_ST.unsafeFreeze(arr)();
        })();
    };
};

Benchmarks

nubBy
---------------
nubBy (101)
mean   = 97.25 μs
stddev = 153.08 μs
min    = 52.47 μs
max    = 3.07 ms
nubBy (10001)
mean   = 9.27 ms
stddev = 3.56 ms
min    = 7.79 ms
max    = 41.68 ms

@JordanMartinez JordanMartinez deleted the speedupNubBy branch September 11, 2021 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants