Opened 4 years ago

Closed 4 years ago

#23410 closed enhancement (fixed)

A performance tweak for MeatAxe

Reported by: SimonKing Owned by:
Priority: minor Milestone: sage-8.1
Component: packages: optional Keywords: meataxe, performance, days88, IMA coding sprints
Cc: Merged in:
Authors: Simon King Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 97d1f20 (Commits, GitHub, GitLab) Commit: 97d1f20ff3640245ee10820a7090604cd0a83afb
Dependencies: Stopgaps:

Status badges

Description

This is in order to split #23352 into smaller chunks.

MeatAxe has global variables FfCurrentRowSizeIo and FfNoc, that by construction always satisfy the equality FfCurrentRowSizeIo==FfTrueRowSize(FfNoc), where FfTrueRowSize is some function.

At some point, when profiling computations involving MeatAxe, I found that the function FfTrueRowSize was called very often. Therefore I suggest to apply yet another patch to MeatAxe, that replaces FfTrueRowSize(FfNoc) by FfCurrentRowSizeIo.

Change History (7)

comment:1 Changed 4 years ago by SimonKing

  • Branch set to u/SimonKing/a_performance_tweak_for_meataxe

comment:2 Changed 4 years ago by SimonKing

  • Authors set to Simon King
  • Commit set to b1aef56c4a1cf9e127ad3ae80b6350719edfe49f
  • Status changed from new to needs_review

New commits:

​b1aef56Add MeatAxe patch that ensures correct value of FfCurrentRowSizeIo

comment:3 follow-up: Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Two details:

  1. It would be good if the .patch file would reference this ticket.
  1. You should bump the version number of the package if you add a patch.

comment:4 Changed 4 years ago by git

  • Commit changed from b1aef56c4a1cf9e127ad3ae80b6350719edfe49f to 97d1f20ff3640245ee10820a7090604cd0a83afb

Branch pushed to git repo; I updated commit sha1. New commits:

​97d1f20Bump meataxe patch level; reference ticket in new patch

comment:5 in reply to: ↑ 3 Changed 4 years ago by SimonKing

  • Status changed from needs_work to needs_review

Replying to jdemeyer:

Two details:

  1. It would be good if the .patch file would reference this ticket.
  1. You should bump the version number of the package if you add a patch.

Done!

comment:6 Changed 4 years ago by tscrim

  • Keywords days88 IMA coding sprints added
  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

Is this patch been submitted to upstream (and planned to)?

comment:7 Changed 4 years ago by vbraun

  • Branch changed from u/SimonKing/a_performance_tweak_for_meataxe to 97d1f20ff3640245ee10820a7090604cd0a83afb
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.