⚡ Bolt: Optimize LaTeX escaping speed#433
Conversation
This change optimizes the `_escape_latex` function in both `app.py` and `resume_generator_latex.py` by extracting the static dictionary mapping and regex compilation out of the function scope and defining them as module-level constants. Since this function is called repeatedly for many fields during resume generation, avoiding recreating the dict and recompiling the regex on every call provides a measurable performance boost. A benchmarking script confirmed this reduced the function execution time by roughly 50%. Co-authored-by: aafre <8656674+aafre@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request optimizes the LaTeX escaping process by hoisting the LATEX_SPECIAL_CHARS dictionary and the LATEX_ESCAPE_PATTERN regex to module-level constants in both app.py and resume_generator_latex.py. This change prevents the overhead of recreating these objects on every function call. Feedback was provided regarding the duplication of this logic across two files, suggesting that these constants and the _escape_latex function be consolidated into a single source of truth to improve maintainability and prevent logic drift.
| LATEX_SPECIAL_CHARS = { | ||
| "\\": r"\textbackslash{}", | ||
| "&": r"\&", | ||
| "%": r"\%", | ||
| "$": r"\$", | ||
| "#": r"\#", | ||
| # "_": r"\_", # Don't escape: used for markdown italic/bold (_text_ and __text__) | ||
| "{": r"\{", | ||
| "}": r"\}", | ||
| # "~": r"\textasciitilde{}", # Don't escape: used for markdown strikethrough (~~text~~) | ||
| "^": r"\textasciicircum{}", | ||
| "<": r"\textless{}", | ||
| ">": r"\textgreater{}", | ||
| "|": r"\textbar{}", | ||
| "-": r"{-}", | ||
| } | ||
|
|
||
| LATEX_ESCAPE_PATTERN = re.compile("|".join(re.escape(key) for key in LATEX_SPECIAL_CHARS.keys())) |
There was a problem hiding this comment.
The LATEX_SPECIAL_CHARS dictionary and LATEX_ESCAPE_PATTERN regex are duplicated between app.py and resume_generator_latex.py. This duplication increases maintenance overhead and the risk of logic drift between the two implementations.
It is recommended to consolidate these constants and the _escape_latex function into a single source of truth (e.g., within resume_generator_latex.py) and import them where needed. The version in resume_generator_latex.py currently contains more descriptive comments which should be retained in the consolidated version.
💡 What: Hoisted
LATEX_SPECIAL_CHARSstatic dictionary andLATEX_ESCAPE_PATTERNregex compilation to module-level constants inapp.pyandresume_generator_latex.py.🎯 Why: The
_escape_latexfunction is called recursively or in loops for almost every string field during PDF/LaTeX resume generation. Recompiling the regex (re.compile) and instantiating the mapping dictionary on every single invocation caused unnecessary overhead and bottlenecked generation speed.📊 Impact: Reduces the execution time of
_escape_latexby approximately 50%, resulting in faster overall PDF/LaTeX generation, especially for content-heavy resumes.🔬 Measurement: Verified locally using
timeitfor 100,000 iterations:Original implementation: ~1.66s
Optimized implementation: ~0.83s
Also ran full Python test suite (
pytest tests/) to ensure no regressions.PR created automatically by Jules for task 1287747391281565954 started by @aafre