-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
275 bun optimized #498
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
275 bun optimized #498
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,67 +1,51 @@ | ||
| import config from "./config.ts"; | ||
| import { save } from "./users.ts"; | ||
| import type { User } from "./types.ts"; | ||
| import type { Config } from "./config.ts"; | ||
|
|
||
|
|
||
| const server = Bun.serve({ | ||
| // Timeout in seconds to match Node.js | ||
| idleTimeout: 60, | ||
| development: false, | ||
| reusePort: true, | ||
| port: (config as Config).appPort, | ||
| port: config.appPort, | ||
| async fetch(req: Request): Promise<Response> { | ||
| const path = new URL(req.url).pathname; | ||
| if (path === "/healthz") return new Response("OK"); | ||
|
|
||
| if (req.method === "GET" && path === "/api/users") { | ||
| const users: User[] = [ | ||
| { | ||
| id: 1, | ||
| name: "David D. Patton", | ||
| address: "1670 Stiles Street", | ||
| phone: "412-578-3857", | ||
| image: `user.png`, | ||
| }, | ||
| { | ||
| id: 2, | ||
| name: "Gary E. Eaton", | ||
| address: "828 Collins Avenue", | ||
| phone: "614-866-1660", | ||
| image: `user.png`, | ||
| }, | ||
| { | ||
| id: 3, | ||
| name: "John J. Fox", | ||
| address: "1895 Columbia Mine Road", | ||
| phone: "304-505-3622", | ||
| image: `user.png`, | ||
| }, | ||
| ]; | ||
| return Response.json(users); | ||
| } | ||
| if (path === "/healthz") return new Response("OK", { status: 200 }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could easily be a https://bun.com/docs/runtime/http/routing For example codeconst server = Bun.serve({
routes: {
"/healthz": new Response("OK", { status: 200 }),
"/api/users": {
GET: async request => {
const users: User[] = [
{ id: 1, name: "David D. Patton", address: "1670 Stiles Street", phone: "412-578-3857", image: `user.png`, },
{ id: 2, name: "Gary E. Eaton", address: "828 Collins Avenue", phone: "614-866-1660", image: `user.png`, },
{ id: 3, name: "John J. Fox", address: "1895 Columbia Mine Road", phone: "304-505-3622", image: `user.png`, },
];
return Response.json(users);
},
// or event simpler, but it feels like possibly cheating:
GET: Response.json([
{ id: 1, name: "David D. Patton", address: "1670 Stiles Street", phone: "412-578-3857", image: `user.png`, },
{ id: 2, name: "Gary E. Eaton", address: "828 Collins Avenue", phone: "614-866-1660", image: `user.png`, },
{ id: 3, name: "John J. Fox", address: "1895 Columbia Mine Road", phone: "304-505-3622", image: `user.png`, },
] satisfies User[]),
POST: async request => {
const incomingUser = await request.json() as User;
const datetime = new Date();
const user: User = {
name: incomingUser.name,
address: incomingUser.address,
phone: incomingUser.phone,
image: `user-bun-${datetime.getTime()}.png`,
createdAt: datetime,
updatedAt: datetime,
};
try {
const record = await save(user);
user.id = record[0].id;
return Response.json(user, { status: 201 });
} catch (error: unknown) {
return Response.json({ message: (error as Error).message }, { status: 400 });
}
},
},
"/*": new Response("Resource not found", { status: 404 }),
},
});
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Static routing is indeed faster, but we keep it around because we aim to simulate real-world scenarios as closely as possible. and similar code structure like go There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For similar, we should at least use routes then (at very least don't use URL class)... but using actual static is pretty real world imo (its not like its a hidden feature of bun) If your talking about the GET request for users, yeah that imo should be DB anyway |
||
| if (path === "/api/users") { | ||
| if (req.method === "GET") { | ||
| const users: User[] = [ | ||
| { id: 1, name: "David D. Patton", address: "1670 Stiles Street", phone: "412-578-3857", image: `user.png`, }, | ||
| { id: 2, name: "Gary E. Eaton", address: "828 Collins Avenue", phone: "614-866-1660", image: `user.png`, }, | ||
| { id: 3, name: "John J. Fox", address: "1895 Columbia Mine Road", phone: "304-505-3622", image: `user.png`, }, | ||
|
solmanter marked this conversation as resolved.
|
||
| ]; | ||
| return Response.json(users); | ||
| } | ||
|
|
||
| if (req.method === "POST" && path === "/api/users") { | ||
| const incomingUser: any = await req.json(); | ||
| const datetime = new Date(); | ||
| const key = `user-bun-${datetime.getTime()}.png`; | ||
| const user: User = { | ||
| ...incomingUser, | ||
| createdAt: datetime, | ||
| updatedAt: datetime, | ||
| image: key, | ||
| }; | ||
| if (req.method === "POST") { | ||
| const incomingUser = await req.json() as User; | ||
| const now = Date.now(); | ||
| const datetime = new Date(now); | ||
|
solmanter marked this conversation as resolved.
|
||
|
|
||
| return save(user) | ||
| .then((record: [{ id: number }]) => { | ||
| const user: User = { | ||
| name: incomingUser.name, | ||
| address: incomingUser.address, | ||
| phone: incomingUser.phone, | ||
| image: `user-bun-${now}.png`, | ||
| createdAt: datetime, | ||
| updatedAt: datetime, | ||
| }; | ||
|
|
||
| try { | ||
| const record = await save(user); | ||
| user.id = record[0].id; | ||
| return Response.json(user, { status: 201 }); | ||
| }) | ||
| .catch((error: Error) => { | ||
| console.error(error); | ||
| return Response.json({ message: error.message }, { status: 400 }); | ||
| }); | ||
| } catch (error: unknown) { | ||
| return Response.json({ message: (error as Error).message }, { status: 400 }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return new Response("Resource not found", { status: 404 }); | ||
| }, | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| import { readFileSync } from "fs"; | ||
| import { load } from "js-yaml"; | ||
| import type { Config } from "./types.ts"; | ||
|
|
||
| const config: Config = load(readFileSync("config.yaml", "utf8")) as Config; | ||
| const config: Config = load(await Bun.file("config.yaml").text()) as Config; | ||
|
|
||
| export type { Config } from './types.ts'; | ||
| export default config; | ||
|
solmanter marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,14 @@ | ||
| import config from "./config.ts"; | ||
| import { SQL } from "bun"; | ||
| import type { Config } from "./types.ts"; | ||
|
|
||
| const { db } = config; | ||
|
|
||
| const sql = new SQL({ | ||
| url: `postgres://${(config as Config).db.user}:${(config as Config).db.password}@${(config as Config).db.host}:5432/${(config as Config).db.database}`, | ||
| max: (config as Config).db.maxConnections, | ||
| url: `postgresql://${db.user}:${db.password}@${db.host}:5432/${db.database}`, | ||
|
solmanter marked this conversation as resolved.
|
||
| max: db.maxConnections, | ||
| idleTimeout: 60, | ||
| connectionTimeout: 30, | ||
| maxLifetime: 1800 | ||
| }); | ||
|
|
||
| export default sql; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,7 @@ | ||
| import sql from "./db.ts"; | ||
| import type { User } from "./types.ts"; | ||
|
|
||
| export async function save({ | ||
| name, | ||
| address, | ||
| phone, | ||
| image, | ||
| createdAt, | ||
| updatedAt, | ||
| }: User): Promise<any> { | ||
| export function save(user: User): Promise<[{ id: number }]> { | ||
| const { name, address, phone, image, createdAt, updatedAt } = user; | ||
| return sql`INSERT INTO bun_app (name, address, phone, image, created_at, updated_at) VALUES (${name}, ${address}, ${phone}, ${image}, ${createdAt}, ${updatedAt}) RETURNING id;`; | ||
| } |
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.
--sql-preconnectis for runtime notbun build, though I think we could do a--compile-exec-argv=--sql-preconnecttype thingBut with how its set via config.yaml and not env vars, I can't see any way this is usable here
Uh oh!
There was an error while loading. Please reload this page.
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.
your are right, that should on runtime
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.
This seems like default behavior anyway, when you deploy an app, it creates all those connections immediately. The more interesting question is why, by the end of the test, it drops the number of connections.
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.
That is interesting to me, i can't find the reason in my tests and benchmarks. It always stayed the same
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.
All this pre connect does is start the connection before javascript code. Which means in theory faster... but as its not set in DATABASE_URL, this flag can't be used
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.
Yes, this is incorrect for this build and it's not important because bun simply ignores this flag. Also, database connections somehow decreased during the bun benchmark. And it done before this update. That's the real problem.