Skip to content

Eliminate overhead caused by CourseId #17

@GenericConfluent

Description

@GenericConfluent

Motivation

Outside the context of a CourseGraph the reasonable way to refer to Courses is by using the subject id (e.g. "CMPUT") and the course number (275) within that subject. It is particularly useful since this is how both the catalogue and people talk about a specific course. The problem is that we often need to duplicate CourseId objects, and since CourseId is !Copy that means a .clone(). The Strings being cloned are relatively small, but as always heap allocation is expensive.

Implementation

The heap allocation slowdown can be addressed by something like tinyvec or tinystr but I want an implementation that also resolves the overhead incurred by converting between a CourseId and a NodeIndex since that is an O(n) operation.

So instead of making CourseId Copy we will make CourseId a wrapper type for a &str (i.e. struct CourseId(String);). Then add HashMap<CourseId, NodeIndex> as a field to HashMap to provide a custom Hash implementation. Note that a second HashMap<NodeIndex, CourseId> is not necessary since the NoteType::Course variant of ntype has Course as associated data which contains a properly formatted CourseId.

All that remains after that is to do some fixes on the usage of CourseId.

Task

  • Update CourseId and provide Hash impl.
  • Update validation logic for the course input.
  • Fix test in src/course_database.rs

For convenience and clean code:

  • impl From<String> for CourseId
  • impl std::ops::Index for Into<CourseId> where type Output = Node;
  • Add method fn edges(&self, id: impl Into<CourseId>) -> Edges<...> to CourseGraph

Metadata

Metadata

Labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions