Skip to content

Cache modules by unique combination of module path and basedir. This …#473

Merged
slavchev merged 1 commit intomasterfrom
slavchev/modules-cache
Jun 10, 2016
Merged

Cache modules by unique combination of module path and basedir. This …#473
slavchev merged 1 commit intomasterfrom
slavchev/modules-cache

Conversation

@slavchev
Copy link
Copy Markdown

@slavchev slavchev commented Jun 8, 2016

…works well for global modules and helps with common requires. Essentially, the currently used map now contains more than one key (path) for a given module

…works well for global modules and helps with common `require`s. Essentially, the currently used map now contains more than one key (path) for a given module
@ns-bot
Copy link
Copy Markdown

ns-bot commented Jun 8, 2016

💔

@atanasovg
Copy link
Copy Markdown
Contributor

I am curious what's the performance gain from this refactoring? Also, is there a way we do all the path calculations on the C++ side and skip the Java marshaling at all?

@slavchev
Copy link
Copy Markdown
Author

slavchev commented Jun 8, 2016

@atanasovg It is up to the application. For the app I am testing with it is ~90ms. It might not help for other apps. The scenarios where this PR helps are:

  • frequently loading global modules (e.g. require("application"))
  • frequently loading relative modules from the same base directory

It won't help much if you load sequentially require("./a/b/c") from basedir and require("./b/c") from basedir/a for example.

@blagoev
Copy link
Copy Markdown
Contributor

blagoev commented Jun 10, 2016

look good

@Plamen5kov
Copy link
Copy Markdown
Contributor

👍

@blagoev
Copy link
Copy Markdown
Contributor

blagoev commented Jun 10, 2016

run ci

@ns-bot
Copy link
Copy Markdown

ns-bot commented Jun 10, 2016

💔

@slavchev slavchev merged commit 4a94cc5 into master Jun 10, 2016
@slavchev slavchev deleted the slavchev/modules-cache branch June 10, 2016 10:48
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.

5 participants