fixed bun miliseconds bug

[DiegoVallejoDev]

Description

bun fixed the bug where performance.now() returned nanosecond instead of milliseconds, removing special bun case beacuse it made the benchmark run for 5,000 seconds

• Tested on Bun 0.1.4 on Debian 11/WSL

Contributing requirements

• [x] I read the contribution guidelines in CONTRIBUTING.md.
• [x] I placed my solution in the correct solution folder.
• [x] I added a README.md with the right badge(s).
• [x] I added a Dockerfile that builds and runs my solution.
• [x] I selected drag-race as the target branch.
• [x] All code herein is licensed compatible with BSD-3.

[rbergen]

@DiefBell Would you like to comment on the observation @DiegoVallejoDev makes, and his proposal to resolve it?

Personally, I have:

• Not been able to reproduce the 5,000 second benchmark runtime that's mentioned
• Noticed that the PrimeJavaScript/solution_1 Docker builds fail, making that the JavaScript implementation are not included in the daily benchmarks. The reason is that the NodeJS version that is to be installed is pinned down to 18.4.0-deb-1nodesource1, which is no longer the 18.x version that NodeSource is providing. Because of issues like this, we don't require individual packages in Dockerfiles to be pinned to a specific version, and we've actually explicitly disabled the corresponding Hadolint rule.

[DiegoVallejoDev]

@rbergen that's interesting what environment are you using?

Also: good catch on the nodejs docker error

[rbergen]

@DiegoVallejoDev I just build and run the Dockerfile, but then with the nodejs version specification removed. That runs fine on my machine. :)

[DiefBell]

You're testing this locally on your own machine, not using the provided Dockerfile I believe?

If you look at the Dockerfile, you'll see that it's using a fixed version of Bun (0.1.2), where the bug is still present. Please update the Dockerfile to use diefenbakerbell/ubuntu-bun:0.1.4, and ensure that you've actually tested using Make and the Dockerfile.

[rbergen]

@DiefBell and @DiegoVallejoDev I'd argue that as long as the Dockerfile included in the solution pins the Bun version to one that requires the exception, it's okay for the exception to be in the code. It may be helpful to add something about 1.4+ versions to the README.md, and the code could even be updated to inspect the Bun version and only apply the exception when needed. However, that's up to you and/or other contributors to decide/implement.

What does need fixing in any case is that the Dockerfile doesn't build at all at the moment. I will open another PR to address this, myself. I'll stick to using diefenbakerbell/ubuntu-bun:0.1.2 as the base image in that. This PR can then keep focusing on the aspect of aligning Bun runtime versions with the solution's code.

[rbergen]

I've opened #854 to address the issue I mentioned in earlier comments.

[DiefBell]

Given Bun is only a few weeks old, I think it would be reasonable to update the base image to 0.1.4 as it's highly unlikely that anyone will be running <0.1.4 ever again. However I noticed a recent PR for Bun has added a GitHub workflow to push versioned official Bun images to Dockerhub, so could be work waiting until the next release of Bun and using the official image.

[rbergen]

That sounds sensible to me. In the meantime, I'll merge #854 as soon as @marghidanu has had an opportunity to review, so that we have the JavaScript implementations back in the daily benchmarks as soon as possible.

[DiefBell]

@DiegoVallejoDev I noticed you haven't updated this in PrimeJavaScript.js. Please ensure that you are actually testing your code with make DIRECTORY=PrimeJavaScript before pushing

[DiefBell]

Have confirmed this runs, lgtm

[DiefBell]

Seems that this needs to rebase onto the up-stream to merge the changes from https://github.com/PlummersSoftwareLLC/Primes/pull/854 in, think I ran it wish a cached Docker image

[DiefBell]

@DiegoVallejoDev have created PR on your fork: https://github.com/DiegoVallejoDev/Primes/pull/1