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

Optimise all the things #100

Open
cb372 opened this issue May 21, 2016 · 3 comments
Open

Optimise all the things #100

cb372 opened this issue May 21, 2016 · 3 comments

Comments

@cb372
Copy link
Owner

cb372 commented May 21, 2016

As we discovered in #82, ScalaCache has quite a high overhead compared to accessing the underlying cache directly. Let's see what we can do to improve this.

@mdedetrich
Copy link
Contributor

I have noticed that you have recently removed scala-logging as a dependency. If you are using slf4j directly, then it will have worse performance than scala-logging. The main reason is that scala-logging (and other logging libraries) usually use an inbuilt macro that will only log if a certain logging level is enabled, i.e. if your log level is INFO and you do logger.debug(...), it won't even call logger.debug.

If you still want to use slf4j directly, then it may be useful to implement a macro so that calls to logCacheHitOrMiss only happen if appropriate logging level is enabled

@cb372
Copy link
Owner Author

cb372 commented May 23, 2016

What the scala-logging macro did was rewrite code like this:

logger.info("yolo")

into code like this:

if (logger.isInfoEnabled) {
  logger.info("yolo")
}

When I removed scala-logging, I just went through the codebase and manually rewrote all the logging calls to look like the latter. It's a bit boilerplate-y, but ScalaCache doesn't do logging in that many places. It wasn't painful enough to make me want to write a macro.

However, you're right that it would be nice to avoid the call to logCacheHitOrMiss if the logger is not enabled at warn level. (The method is too big for HotSpot to inline for us.) Could do this manually or with a macro.

@mdedetrich
Copy link
Contributor

However, you're right that it would be nice to avoid the call to logCacheHitOrMiss if the logger is not enabled at warn level. (The method is too big for HotSpot to inline for us.) Could do this manually or with a macro.

I can do this in #103 If you wish

mdedetrich added a commit to mdedetrich/scalacache that referenced this issue May 31, 2016
…f within the function. Attempt to improve performance cb372#100
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

No branches or pull requests

2 participants