Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better normalization cache #738

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

Conversation

masylum
Copy link

@masylum masylum commented Jun 17, 2024

They key seems to be too specific. Specially by using the prop, which
basically makes it redudant to cache tokens that are found in different
props. The goal of that cache seems to be to trade memory for time, but
right now seems to be storing equal computations in different keys which
basically is inefficient. The only thing that the prop is needed for is
the stemmerSkipProperties.

Copy link

vercel bot commented Jun 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
orama-docs 🛑 Canceled (Inspect) Jun 17, 2024 10:23am

They key seems to be too specific. Specially by using the prop, which
basically makes it redudant to cache tokens that are found in different
props. The goal of that cache seems to be to trade memory for time, but
right now seems to be storing equal computations in different keys which
basically is inefficient. The only thing that the prop is needed for is
the `stemmerSkipProperties`.
@micheleriva
Copy link
Member

Hi there! Thank you so much for your PR. Were you able to run the tests locally?

@masylum
Copy link
Author

masylum commented Jul 24, 2024

hey, not really! I also found another positive side-effect of simplifying the normalization cache. Right now, these are always cache misses, which means that using the highlight plugin is pretty slow: https://github.com/askorama/orama/blob/7512ca936ffa1543a0e267ae9e9b3d4187be0bdd/packages/plugin-match-highlight/src/index.ts#L74

@masylum
Copy link
Author

masylum commented Jul 24, 2024

I can't run tests locally, I get this when doing npm install

npm ERR! Unsupported URL Type "workspace:": workspace:*

@masylum
Copy link
Author

masylum commented Jul 24, 2024

so it looks like the regression was introduced here: https://github.com/askorama/orama/pull/350/files

I'm currently working it around doing my own tokenizer, but it's not ideal

@micheleriva
Copy link
Member

I can't run tests locally, I get this when doing npm install

You should use pnpm! pnpm install works just fine :)

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