Skip to content
This repository has been archived by the owner on Aug 28, 2021. It is now read-only.

Optimize struct type validation. The regexp's show in profiles. #2866

Closed
ghost opened this issue Nov 30, 2016 · 9 comments
Closed

Optimize struct type validation. The regexp's show in profiles. #2866

ghost opened this issue Nov 30, 2016 · 9 comments

Comments

@ghost
Copy link

ghost commented Nov 30, 2016

While doing something else, I noticed that sync appeared to have gotten slower. I bisected it and discovered that 5e901b0 regressed syncing ~300MB of sf_scrime data from ldb to ldb by ~ 100%.

After some short digging, I'm fairly sure it's because of the addition of type validation to the validating_batch_sink.

Thinking about it some, I think it's an easy fix because of the type cache. Basically we're validating over and over again exactly the same in-memory type graph (the type cache ensures that identical types are interned). we just need to find a way to only do it once per type.

@ghost ghost assigned arv Nov 30, 2016
@ghost
Copy link
Author

ghost commented Nov 30, 2016

cc @aboodman to set priority

@ghost
Copy link
Author

ghost commented Nov 30, 2016

this essentially affects writing of data to a remote server (which logically includes syncing to a local ldb store)

@aboodman
Copy link
Contributor

aboodman commented Dec 1, 2016

Let's just drive it by perf of attic. For example I just asked @arv to estimate difficulty of #2869 because it badly affects attic ui perf. If we find that this bug also affects perf, then fix. Otherwise no.

@aboodman
Copy link
Contributor

aboodman commented Dec 1, 2016

IOW, not a prio right now.

@aboodman aboodman unassigned arv Dec 1, 2016
@ghost ghost changed the title struct type validation regressed noms sync Remove struct type validation Feb 11, 2017
@ghost
Copy link
Author

ghost commented Feb 11, 2017

Blocked by #3123

@ghost ghost mentioned this issue Feb 11, 2017
@ghost ghost changed the title Remove struct type validation Optimize struct type validation. The regexp's show in profiles. Apr 10, 2017
@ghost ghost removed the Format label Aug 15, 2017
@arv
Copy link
Contributor

arv commented Sep 21, 2017

Is this still valid?

@ghost
Copy link
Author

ghost commented Sep 22, 2017

I looked recently and I kinda looked like it was. The problem is that the regex is expensive.

@arv
Copy link
Contributor

arv commented Sep 22, 2017

Which regexp is this? The one that validates the name?

@ghost
Copy link
Author

ghost commented Sep 22, 2017

Yup. Struct and field name.

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

No branches or pull requests

2 participants