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.

Sign up for GitHub

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

[IMP] reactivity: replace sets with small arrays for performance #1581

Merged
merged 1 commit into from
Jan 12, 2024
Merged

[IMP] reactivity: replace sets with small arrays for performance #1581

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/runtime/reactivity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ type CollectionRawType = "Set" | "Map" | "WeakMap";
const objectToString = Object.prototype.toString;
const objectHasOwnProperty = Object.prototype.hasOwnProperty;

const SUPPORTED_RAW_TYPES = new Set(["Object", "Array", "Set", "Map", "WeakMap"]);
const COLLECTION_RAWTYPES = new Set(["Set", "Map", "WeakMap"]);
// Use arrays because Array.includes is faster than Set.has for small arrays
const SUPPORTED_RAW_TYPES = ["Object", "Array", "Set", "Map", "WeakMap"];
const COLLECTION_RAW_TYPES = ["Set", "Map", "WeakMap"];

/**
* extract "RawType" from strings like "[object RawType]" => this lets us ignore
Expand All @@ -45,7 +46,7 @@ function canBeMadeReactive(value: any): boolean {
if (typeof value !== "object") {
return false;
}
return SUPPORTED_RAW_TYPES.has(rawType(value));
return SUPPORTED_RAW_TYPES.includes(rawType(value));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at this point, maybe it's even faster to simply check for equality:

const type = rawType(value);
return type === "Object" || type === "Array" || type === "Set" || type === "Map" || type === "WeakMap";

Same for collections. what do you think? @sdegueldre

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried with an inline switch previously when experimenting but perf was about the same, I'm guessing this is already very easy to JIT efficiently but may require more exploration.

}
/**
* Creates a reactive from the given object/callback if possible and returns it,
Expand Down Expand Up @@ -220,7 +221,7 @@ export function reactive<T extends Target>(target: T, callback: Callback = NO_CA
const reactivesForTarget = reactiveCache.get(target)!;
if (!reactivesForTarget.has(callback)) {
const targetRawType = rawType(target);
const handler = COLLECTION_RAWTYPES.has(targetRawType)
const handler = COLLECTION_RAW_TYPES.includes(targetRawType)
? collectionsProxyHandler(target as Collection, callback, targetRawType as CollectionRawType)
: basicProxyHandler<T>(callback);
const proxy = new Proxy(target, handler as ProxyHandler<T>) as Reactive<T>;
Expand Down
Loading