hoeck / simple-runtypes

Small, efficient and extendable runtype library for Typescript
MIT License
114 stars 5 forks source link

improve performance to find unknown keys for record runtype #20

Closed pabra closed 3 years ago

pabra commented 3 years ago

in moltar/typescript-runtime-type-benchmarks

before: ~2_200_000 ops/sec
after:  ~2_300_000 ops/sec
hoeck commented 3 years ago

oh that looks good, nice catch.

In case you're in the mood for some perf probing, could you please check whether the following gains any additional ops?:

pabra commented 3 years ago
  1. that actually made it slower by ~300_000 - that's what I did:

    index 9ec0976..ed6c8fc 100644
    --- a/src/record.ts
    +++ b/src/record.ts
    @@ -17,13 +17,14 @@ function internalRecord<T extends object>(
     ): Runtype<T> {
       const isPure = Object.values(typemap).every((t: any) => isPureRuntype(t))
       const copyObject = sloppy || !isPure
    +  const typemapKeys = Object.keys(typemap) as (keyof T)[]
       const getUnknownKeys = (o: object) => {
         const keys = []
         for (const k in o) {
           if (!Object.prototype.hasOwnProperty.call(o, k)) {
             continue
           }
    -      if (!Object.prototype.hasOwnProperty.call(typemap, k)) {
    +      if (typemapKeys.indexOf(k as any) === -1) {
             keys.push(k)
           }
         }
    @@ -41,12 +42,13 @@ function internalRecord<T extends object>(
         // return a different object - otherwise return value as is
         const res = copyObject ? ({} as T) : (o as T)
    
    -    for (const k in typemap) {
    +    for (let i = 0; i < typemapKeys.length; i++) {
    +      const k = typemapKeys[i]
           // nested types should always fail with explicit `Fail` so we can add additional data
           const value = (typemap[k] as InternalRuntype)(o[k], failSymbol)
    
           if (isFail(value)) {
    -        return propagateFail(failOrThrow, value, v, k)
    +        return propagateFail(failOrThrow, value, v, k as any)
           }
    
           if (copyObject) {
    @@ -71,9 +73,10 @@ function internalRecord<T extends object>(
    
       // fields metadata to implement combinators like (discriminated) unions,
       // pick, omit and intersection
    -  const fields: { [key: string]: any } = {}
    +  const fields = {} as { [K in keyof T]: any }
    
    -  for (const k in typemap) {
    +  for (let i = 0; i < typemapKeys.length; i++) {
    +    const k = typemapKeys[i]
         fields[k] = typemap[k]
       }

    I also write typemapKeys = Object.keys(typemap) as (keyof T)[] as loop for(const key in typemap) to get the keys. But the result is about the same.

  2. I do not notice any difference.

  3. I don't know what you mean.

hoeck commented 3 years ago

if (typemapKeys.indexOf(k as any) === -1) { that of course slows it down.

I was speculating that object-key iteration (for (const key in object)) is maybe slower than a plain for-index-in-array loop as the latter does not involve any object-runtime magic (like finding the objects symbols).

Thanks for trying, I may just look into it and you already found a good perf improvemend with the getunknownkeys stuff!

pabra commented 3 years ago

Now that I think about it, of course indexOf makes it slower. But with that line changed back, it's only slower by about ~200_000 ops.

hoeck commented 3 years ago

I did manage to get a ~30% speedup (from around 1.8m to 2.5m ops) with this: https://github.com/hoeck/simple-runtypes/commit/14bac166c2204579bc69769d33a4fcf3d88b51bd (basically your unknown-keys fix inlined plus the two suggestions from me with the explicit loop over the cached array of typemap keys and inlining the object type check).

Thanks m8! That makes simple-runtypes in the moltar benchmark as fast or slightly faster than valita with strict checks turned on :grin: