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
Add benchmark for net/http #1289
Conversation
1cbda75
to
e2053be
Compare
I updated the branch to use submodules. But I'm getting an unexpected error now
|
Seems like for some reason it starts thing a relative module is a remote module. Just posting this as I am digging into it, with the debug flag:
|
It is going down this path: Lines 262 to 263 in 568ac0c
I added a
So because it believes it is the |
@kitsonk Yikes, I found the real reason. Instead, it is caused by these lines: Lines 152 to 154 in 568ac0c
and with this: Lines 137 to 139 in 568ac0c
We don't have .mime files here, so it crashes
|
@kevinkassimo while that certainly might also be an issue, the issue of the containing file changing is being mis-understood in |
@kitsonk I have tried fixed these lines and seems benchmark is working now.
I added a bunch of Line 153 in 568ac0c
(You can try add logging immediately before and after this line. There is no panic because the extension guessing mechanism consumes the error...)
I'm making a PR for this |
@kevinkassimo yeah, I see what you are saying, but it shouldn't even be looking on the network for the file... I can see how the lack of |
@kitsonk I might be understanding it wrong, but I still believe it won't look on the network if a locally cached file is present. Lines 322 to 342 in 568ac0c
And thus converting back to a local cached path again... With the file exist at the path no remote fetch would happen... The thing that actually makes me worried instead is that we are not currently checking if files with added suffixes exist in the cache. However, that is another issue. |
@kevinkassimo yeah, checking in |
ee8230b
to
158c1bc
Compare
Rebased on top of Kevin's #1291. It's working locally for me now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thanks @bartlomieju @kitsonk @kevinkassimo !
Benchmark for HTTP server from https://github.com/denoland/net
Results from my machine: