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.

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 flag to prioritize glob of files when searching for types #60

Closed
wants to merge 1 commit into from

Conversation

bywo
Copy link
Contributor

@bywo bywo commented Jan 24, 2019

--files 'src/resources/**' argument will search those files first for the given type.

Shaves off less than a second in my usage, but it's still an improvement:

~/d/api ❯❯❯ time node ../ts-json-schema-generator/bin/ts-json-schema-generator --path ./tsconfig.json --type 'UserConvertTypePayload' --skip-type-check
10.38s user 0.65s system 166% cpu 6.609 total
~/d/api ❯❯❯ time node ../ts-json-schema-generator/bin/ts-json-schema-generator --path ./tsconfig.json --type 'UserConvertTypePayload' --skip-type-check --files 'src/resources/**'
9.65s user 0.62s system 168% cpu 6.085 total

resolves #59

@domoritz
Copy link
Member

Just wondering, if we just prioritize files that are not in node_modules, do we get the same benefit (but we avoid having to add a new command line option)? Let's try this before merging. Thanks!

@bywo
Copy link
Contributor Author

bywo commented Jan 24, 2019

Yea I think we'd get a benefit, but it wouldn't allow you to be as specific about which files to target first. For example, if you knew exactly which file the type lived in, you'd see a bigger benefit than if you just deprioritized node_modules.

I could also change the default algorithm to deprioritize node_modules so everyone gets the benefit.

@domoritz
Copy link
Member

domoritz commented Jan 24, 2019

I could also change the default algorithm to deprioritize node_modules so everyone gets the benefit.

Let's try that first in a separate PR and measure the impact. I have a hunch that scanning node_modules takes a majority of the time.

Thanks for helping with this and bearing with my obsession to measure and get data before we make a decision.

@bywo
Copy link
Contributor Author

bywo commented Jan 24, 2019

check this out: #61

@domoritz
Copy link
Member

Let's re-evaluate after #61 is merged.

@domoritz
Copy link
Member

I'm closing this for now but happy to review another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SchemaGenerator optimization
2 participants