Skip to content
This repository has been archived by the owner on Sep 30, 2023. It is now read-only.

Feat: improve memory usage / allow for in-memory storage of all entries to be optional (WIP) #301

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mistakia
Copy link
Contributor

@mistakia mistakia commented Apr 22, 2020

⚠️ Note: This PR is a WIP and not ready for review. I'm in the process of cleaning up a fork that I have been using in a proof of concept application. My hope is to minimize the amount of breaking changes further.

Description

Currently, all entries (and all the data in it) are kept in memory. The goal of this PR is to make this optional in order to improve load times and performance on memory-limited environments when interacting with larger/longer log sets.

Related: #136, #203, #239

Breaking Changes

The following getters and methods become async / return promises.

  • log.values
  • log.tails
  • log.tailHashes
  • log.get()
  • log.traverse()
  • log.iterator()
  • log.toString()
  • log.toSnapshot()

Benchmarking

Before
wip

After
wip

Tradeoffs

wip

TODO

  • update tests
  • write new tests
  • update benchmarks / benchmarking / document performance tradeoffs
  • adjust entryIndex to allow caching of all entries by default
  • document breaking changes
  • update method comments
  • squash / clean up commits
  • remove forked orbit-db-io

@aphelionz
Copy link
Contributor

Excited about this if it turns out to work! Would you mind though making this a draft PR until it's ready?

@mistakia mistakia marked this pull request as draft May 7, 2020 01:51
@mistakia mistakia force-pushed the proto/record branch 6 times, most recently from 9f0efb9 to 39b893d Compare May 12, 2020 14:08
@aphelionz
Copy link
Contributor

How are you feeling about this one @mistakia ? :)

@mistakia
Copy link
Contributor Author

Feeling like I need to get back in the game! I plan on coming back to this in the not so distant future.

@aphelionz
Copy link
Contributor

Ok, I'll leave it open then 👍

@aphelionz
Copy link
Contributor

@mistakia the not too distant future is upon us :D hope you are happy + healthy in these trying times.

If you want to hop back in I can totally support you. Did you see I finally merged #307 ?

@mistakia mistakia force-pushed the proto/record branch 4 times, most recently from 2d3f048 to 44b9a77 Compare January 15, 2021 04:20
@mistakia
Copy link
Contributor Author

mistakia commented Jan 20, 2021

Quick status update of what I think is left. Merge the following into ipfs-log master:

After these two I can dig deeper in the benchmarks to make improvements (i.e. analyze various LRU libs)

  • update log.spec tests
  • what to do with errors when loading an entry from a multihash
  • document breaking changes
  • update docs

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants