Skip to content

Conversation

mlovic
Copy link
Contributor

@mlovic mlovic commented Jan 22, 2018

See Redis LTRIM command docs here.

Commit description:

This implementation will only execute 1 to Redis command, instead of
2n (n being the total number of elements to trim from the list).

The operation will still take O(n) time, but the constants will be
much better, since only 1 network round trip is required.

A small improvement, but I happened to notice while looking around the code. Admittedly, it doesn't seem this method is currently used with a lot of data, but I think it's easy to imagine it having a significant impact on performance if used differently in the future.

I'd be happy to submit a benchmark if that would be helpful :).

This implementation will only execute 1 to Redis command, instead of
2n (n being the total number of elements to trim from the list).

The operation will still take O(n) time, but the constants will be
much better, since only 1 network round trip is required.
@andrew
Copy link
Member

andrew commented Jan 22, 2018

Thanks for the contribution, does this require a particular version of redis or is it a pretty standard method?

@mlovic
Copy link
Contributor Author

mlovic commented Jan 23, 2018

It's been available since Redis 1.0.0 (source), the same as LLEN and RPOP which are used in the current implementation.

@andrew
Copy link
Member

andrew commented Jan 23, 2018

Sweet!

@andrew andrew merged commit d8b00b8 into splitrb:master Jan 23, 2018
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