oli-obk / pandoc-ast

35 stars 11 forks source link

Pre-compile filter to improve compile speed #1

Closed Rufflewind closed 7 years ago

Rufflewind commented 7 years ago

Instantiate filter in the library so the user doesn't have to instantiate its contents themselves, which would otherwise eat up a lot of compilation time due to serde (https://github.com/serde-rs/json/issues/313).

This is purely an optimization for faster compilation.

dtolnay commented 7 years ago

Mentioning @oli-obk because they aren't watching this repo.

oli-obk commented 7 years ago

I'd prefer to create two functions: one for converting string -> pandoc and one the other way. Then call those instead of doing an option dance.

So weird. I'm not watching half my repos. I though new repos got automatically watched...

Rufflewind commented 7 years ago

Do you want the error-checking included as part of the &str -> Pandoc function (in which case it might be good idea to add an Error type), or leave them inside filter? Should the &str -> Value -> String -> Pandoc dance be part of the &str -> Pandoc function, or part of filter?


Some benchmarks on the contributions to compile time:

The percentages are relative to the total compile time for the test case in https://github.com/serde-rs/json/issues/313#issue-225849316. The absolute timings are for debug mode (total 10.74 s).

oli-obk commented 7 years ago

Some benchmarks on the contributions to compile time:

Oh, that's quite some time for such a small piece of code...

Do you want the error-checking included as part of the &str -> Pandoc function (in which case it might be good idea to add an Error type), or leave them inside filter?

I'd just leave it all as panics, We can fix this some time in the future by implementing Deserialize and Serialize manually. Just make the methods private for now.

Should the &str -> Value -> String -> Pandoc dance be part of the &str -> Pandoc function, or part of filter?

Part of &str -> Pandoc as right now is great.

Rufflewind commented 7 years ago

Updated.

oli-obk commented 7 years ago

Thanks!

oli-obk commented 7 years ago

I published these changes together with an update to serde 1.0 as pandoc_ast 0.7